Eliminate dead branches around typeof comparisons

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs

@@ -22,11 +22,13 @@ public class SubstitutedILProvider : ILProvider
     {
         private readonly ILProvider _nestedILProvider;
         private readonly SubstitutionProvider _substitutionProvider;
+        private readonly DevirtualizationManager _devirtualizationManager;
 
-        public SubstitutedILProvider(ILProvider nestedILProvider, SubstitutionProvider substitutionProvider)
+        public SubstitutedILProvider(ILProvider nestedILProvider, SubstitutionProvider substitutionProvider, DevirtualizationManager devirtualizationManager)
         {
             _nestedILProvider = nestedILProvider;
             _substitutionProvider = substitutionProvider;
+            _devirtualizationManager = devirtualizationManager;
         }
 
         public override MethodIL GetMethodIL(MethodDesc method)
@@ -858,7 +860,26 @@ private static bool TryExpandTypeIs(MethodIL methodIL, byte[] body, OpcodeFlags[
             return true;
         }
 
-        private static bool TryExpandTypeEquality(MethodIL methodIL, byte[] body, OpcodeFlags[] flags, int offset, string op, out int constant)
+        private bool TryExpandTypeEquality(MethodIL methodIL, byte[] body, OpcodeFlags[] flags, int offset, string op, out int constant)
+        {
+            if (TryExpandTypeEquality_TokenToken(methodIL, body, flags, offset, out constant)
+                || TryExpandTypeEquality_TokenOther(methodIL, body, flags, offset, 1, expectGetType: false, out constant)
+                || TryExpandTypeEquality_TokenOther(methodIL, body, flags, offset, 2, expectGetType: false, out constant)
+                || TryExpandTypeEquality_TokenOther(methodIL, body, flags, offset, 3, expectGetType: false, out constant)
+                || TryExpandTypeEquality_TokenOther(methodIL, body, flags, offset, 1, expectGetType: true, out constant)
+                || TryExpandTypeEquality_TokenOther(methodIL, body, flags, offset, 2, expectGetType: true, out constant)
+                || TryExpandTypeEquality_TokenOther(methodIL, body, flags, offset, 3, expectGetType: true, out constant))
+            {
+                if (op == "op_Inequality")
+                    constant ^= 1;
+
+                return true;
+            }
+
+            return false;
+        }
+
+        private static bool TryExpandTypeEquality_TokenToken(MethodIL methodIL, byte[] body, OpcodeFlags[] flags, int offset, out int constant)
         {
             // We expect to see a sequence:
             // ldtoken Foo
@@ -906,9 +927,104 @@ private static bool TryExpandTypeEquality(MethodIL methodIL, byte[] body, Opcode
 
             constant = equality.Value ? 1 : 0;
 
-            if (op == "op_Inequality")
-                constant ^= 1;
+            return true;
+        }
+
+        private bool TryExpandTypeEquality_TokenOther(MethodIL methodIL, byte[] body, OpcodeFlags[] flags, int offset, int ldInstructionSize, bool expectGetType, out int constant)
+        {
+            // We expect to see a sequence:
+            // ldtoken Foo
+            // call GetTypeFromHandle
+            // ldloc.X/ldloc_s X/ldarg.X/ldarg_s X
+            // [optional] call Object.GetType
+            // -> offset points here
+            //
+            // The ldtoken part can potentially be in the second argument position
+
+            constant = 0;
+            int sequenceLength = 5 + 5 + ldInstructionSize + (expectGetType ? 5 : 0);
+            if (offset < sequenceLength)
+                return false;
+
+            if ((flags[offset - sequenceLength] & OpcodeFlags.InstructionStart) == 0)
+                return false;
+
+            ILReader reader = new ILReader(body, offset - sequenceLength);
+
+            TypeDesc knownType = null;
+
+            // Is the ldtoken in the first position?
+            if (reader.PeekILOpcode() == ILOpcode.ldtoken)
+            {
+                knownType = ReadLdToken(ref reader, methodIL, flags);
+                if (knownType == null)
+                    return false;
+
+                if (!ReadGetTypeFromHandle(ref reader, methodIL, flags))
+                    return false;
+            }
+
+            ILOpcode opcode = reader.ReadILOpcode();
+            if (ldInstructionSize == 1 && opcode is (>= ILOpcode.ldloc_0 and <= ILOpcode.ldloc_3) or (>= ILOpcode.ldarg_0 and <= ILOpcode.ldarg_3))
+            {
+                // Nothing to read
+            }
+            else if (ldInstructionSize == 2 && opcode is ILOpcode.ldloc_s or ILOpcode.ldarg_s)
+            {
+                reader.ReadILByte();
+            }
+            else if (ldInstructionSize == 3 && opcode is ILOpcode.ldloc or ILOpcode.ldarg)
+            {
+                reader.ReadILUInt16();
+            }
+            else
+            {
+                return false;
+            }
+
+            if ((flags[reader.Offset] & OpcodeFlags.BasicBlockStart) != 0)
+                return false;
+
+            if (expectGetType)
+            {
+                if (reader.ReadILOpcode() is not ILOpcode.callvirt and not ILOpcode.call)
+                    return false;
 
+                // We don't actually mind if this is not Object.GetType
+                reader.ReadILToken();
+
+                if ((flags[reader.Offset] & OpcodeFlags.BasicBlockStart) != 0)
+                    return false;
+            }
+
+            // If the ldtoken wasn't in the first position, it must be in the other
+            if (knownType == null)
+            {
+                knownType = ReadLdToken(ref reader, methodIL, flags);
+                if (knownType == null)
+                    return false;
+
+                if (!ReadGetTypeFromHandle(ref reader, methodIL, flags))
+                    return false;
+            }
+
+            // No value in making this work for definitions
+            if (knownType.IsGenericDefinition)
+                return false;
+
+            // Dataflow runs on top of uninstantiated IL and we can't answer some questions there.
+            // Unfortunately this means dataflow will still see code that the rest of the system
+            // might have optimized away. It should not be a problem in practice.
+            if (knownType.ContainsSignatureVariables())
+                return false;
+
+            if (knownType.IsCanonicalDefinitionType(CanonicalFormKind.Any))
+                return false;
+
+            if (_devirtualizationManager.CanReferenceConstructedTypeOrCanonicalFormOfType(knownType))
+                return false;
+
+            constant = 0;
             return true;
         }
 

src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs

@@ -113,7 +113,7 @@ public ILScanResults Trim (ILCompilerOptions options, TrimmingCustomizations? cu
 			}
 
 			SubstitutionProvider substitutionProvider = new SubstitutionProvider(logger, featureSwitches, substitutions);
-			ilProvider = new SubstitutedILProvider(ilProvider, substitutionProvider);
+			ilProvider = new SubstitutedILProvider(ilProvider, substitutionProvider, new DevirtualizationManager());
 
 			CompilerGeneratedState compilerGeneratedState = new CompilerGeneratedState (ilProvider, logger);
 

src/coreclr/tools/aot/ILCompiler/Program.cs

@@ -378,7 +378,8 @@ public int Run()
             }
 
             SubstitutionProvider substitutionProvider = new SubstitutionProvider(logger, featureSwitches, substitutions);
-            ilProvider = new SubstitutedILProvider(ilProvider, substitutionProvider);
+            ILProvider unsubstitutedILProvider = ilProvider;
+            ilProvider = new SubstitutedILProvider(ilProvider, substitutionProvider, new DevirtualizationManager());
 
             CompilerGeneratedState compilerGeneratedState = new CompilerGeneratedState(ilProvider, logger);
 
@@ -492,10 +493,17 @@ void RunScanner()
                 if (scanDgmlLogFileName != null)
                     scanResults.WriteDependencyLog(scanDgmlLogFileName);
 
+                DevirtualizationManager devirtualizationManager = scanResults.GetDevirtualizationManager();
+
                 metadataManager = ((UsageBasedMetadataManager)metadataManager).ToAnalysisBasedMetadataManager();
 
                 interopStubManager = scanResults.GetInteropStubManager(interopStateManager, pinvokePolicy);
 
+                ilProvider = new SubstitutedILProvider(unsubstitutedILProvider, substitutionProvider, devirtualizationManager);
+
+                // Use a more precise IL provider that uses whole program analysis for dead branch elimination
+                builder.UseILProvider(ilProvider);
+
                 // If we have a scanner, feed the vtable analysis results to the compilation.
                 // This could be a command line switch if we really wanted to.
                 builder.UseVTableSliceProvider(scanResults.GetVTableLayoutInfo());
@@ -507,7 +515,7 @@ void RunScanner()
                 // If we have a scanner, we can drive devirtualization using the information
                 // we collected at scanning time (effectively sealing unsealed types if possible).
                 // This could be a command line switch if we really wanted to.
-                builder.UseDevirtualizationManager(scanResults.GetDevirtualizationManager());
+                builder.UseDevirtualizationManager(devirtualizationManager);
 
                 // If we use the scanner's result, we need to consult it to drive inlining.
                 // This prevents e.g. devirtualizing and inlining methods on types that were

src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs

@@ -346,19 +346,44 @@ sealed class Gen<T> { }
 
         sealed class Never { }
 
-        static Type s_type = null;
+        class Never2 { }
+        class Canary2 { }
+        class Never3 { }
+        class Canary3 { }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static Type GetTheType() => null;
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static object GetTheObject() => new object();
+
+        static volatile object s_sink;
 
         public static void Run()
         {
             // This was asserting the BCL because Never would not have reflection metadata
             // despite the typeof
-            Console.WriteLine(s_type == typeof(Never));
+            Console.WriteLine(GetTheType() == typeof(Never));
 
             // This was a compiler crash
             Console.WriteLine(typeof(object) == typeof(Gen<>));
 
 #if !DEBUG
             ThrowIfPresent(typeof(TestTypeEquals), nameof(Never));
+
+            Type someType = GetTheType();
+            if (someType == typeof(Never2))
+            {
+                s_sink = new Canary2();
+            }
+            ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Canary2));
+
+            object someObject = GetTheObject();
+            if (someObject.GetType() == typeof(Never3))
+            {
+                s_sink = new Canary3();
+            }
+            ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Canary3));
 #endif
         }
     }