Avoid scanning isinst checks when building whole program view (#104148)

Before this PR, we were somewhat able to eliminate dead typeof checks such as:

```csharp
if (someType is Foo f)
{
    ExpensiveMethod();
}
```

This work was done in #99761.

However, the optimization only happened during codegen. This meant that when building the whole program view, we'd still look at `ExpensiveMethod` and whatever damage this caused to the whole program view was permanent.

With this PR, the scanner now becomes aware of the optimization we do during codegen and tries to defer injecting dependencies until we will need them. Despite RyuJIT already doing this optimization, I'm also doing it in the SubstitutedILProvider because this optimization _must_ happen, or we'll get codegen failures.

With this change, we detect the conditional branch, and generate whatever dependencies from the basic block as conditional. That way scanning can fully skip scanning `ExpensiveMethod` and the subsequent optimization will ensure the missed scanning will not cause issues at codegen time.

Author: MichalStrehovsky

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

@@ -253,6 +253,7 @@ public MethodIL GetMethodILWithInlinedSubstitutions(MethodIL method)
                     continue;
 
                 TypeEqualityPatternAnalyzer typeEqualityAnalyzer = default;
+                IsInstCheckPatternAnalyzer isInstCheckAnalyzer = default;
 
                 ILReader reader = new ILReader(methodBytes, offset);
                 while (reader.HasNext)
@@ -262,6 +263,7 @@ public MethodIL GetMethodILWithInlinedSubstitutions(MethodIL method)
                     ILOpcode opcode = reader.ReadILOpcode();
 
                     typeEqualityAnalyzer.Advance(opcode, reader, method);
+                    isInstCheckAnalyzer.Advance(opcode, reader, method);
 
                     // Mark any applicable EH blocks
                     foreach (ILExceptionRegion ehRegion in ehRegions)
@@ -302,7 +304,8 @@ public MethodIL GetMethodILWithInlinedSubstitutions(MethodIL method)
                     {
                         int destination = reader.ReadBranchDestination(opcode);
                         if (!TryGetConstantArgument(method, methodBytes, flags, offset, 0, out int constant)
-                            && !TryExpandTypeEquality(typeEqualityAnalyzer, method, out constant))
+                            && !TryExpandTypeEquality(typeEqualityAnalyzer, method, out constant)
+                            && !TryExpandIsInst(isInstCheckAnalyzer, method, out constant))
                         {
                             // Can't get the constant - both branches are live.
                             offsetsToVisit.Push(destination);
@@ -931,6 +934,35 @@ private bool TryExpandTypeEquality(in TypeEqualityPatternAnalyzer analyzer, Meth
             return true;
         }
 
+        private bool TryExpandIsInst(in IsInstCheckPatternAnalyzer analyzer, MethodIL methodIL, out int constant)
+        {
+            constant = 0;
+            if (!analyzer.IsIsInstBranch)
+                return false;
+
+            var type = (TypeDesc)methodIL.GetObject(analyzer.Token);
+
+            // 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 (type.ContainsSignatureVariables())
+                return false;
+
+            if (type.IsCanonicalDefinitionType(CanonicalFormKind.Any))
+                return false;
+
+            // We don't track types without a constructed MethodTable very well.
+            if (!ConstructedEETypeNode.CreationAllowed(type))
+                return false;
+
+            if (_devirtualizationManager.CanReferenceConstructedTypeOrCanonicalFormOfType(type.NormalizeInstantiation()))
+                return false;
+
+            constant = 0;
+
+            return true;
+        }
+
         private static TypeDesc ReadLdToken(ref ILReader reader, MethodIL methodIL, OpcodeFlags[] flags)
         {
             ILOpcode opcode = reader.ReadILOpcode();

src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs

@@ -35,6 +35,7 @@ internal partial class ILImporter
         private readonly byte[] _ilBytes;
 
         private TypeEqualityPatternAnalyzer _typeEqualityPatternAnalyzer;
+        private IsInstCheckPatternAnalyzer _isInstCheckPatternAnalyzer;
 
         private sealed class BasicBlock
         {
@@ -263,11 +264,13 @@ private void StartImportingBasicBlock(BasicBlock basicBlock)
             }
 
             _typeEqualityPatternAnalyzer = default;
+            _isInstCheckPatternAnalyzer = default;
         }
 
         partial void StartImportingInstruction(ILOpcode opcode)
         {
             _typeEqualityPatternAnalyzer.Advance(opcode, new ILReader(_ilBytes, _currentOffset), _methodIL);
+            _isInstCheckPatternAnalyzer.Advance(opcode, new ILReader(_ilBytes, _currentOffset), _methodIL);
         }
 
         private void EndImportingInstruction()
@@ -837,6 +840,16 @@ private void ImportBranch(ILOpcode opcode, BasicBlock target, BasicBlock fallthr
                 }
             }
 
+            if (opcode == ILOpcode.brfalse && _isInstCheckPatternAnalyzer.IsIsInstBranch)
+            {
+                TypeDesc isinstCheckType = (TypeDesc)_canonMethodIL.GetObject(_isInstCheckPatternAnalyzer.Token);
+                if (ConstructedEETypeNode.CreationAllowed(isinstCheckType)
+                    && !isinstCheckType.ConvertToCanonForm(CanonicalFormKind.Specific).IsCanonicalSubtype(CanonicalFormKind.Any))
+                {
+                    condition = _factory.MaximallyConstructableType(isinstCheckType);
+                }
+            }
+
             ImportFallthrough(target);
 
             if (fallthrough != null)

src/coreclr/tools/aot/ILCompiler.Compiler/IL/IsInstCheckPatternAnalyzer.cs

@@ -0,0 +1,129 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+
+using Internal.IL;
+using Internal.TypeSystem;
+
+namespace ILCompiler
+{
+    /// <summary>
+    /// Simple state machine to analyze IL sequences that represent isinst checks.
+    /// </summary>
+    internal struct IsInstCheckPatternAnalyzer
+    {
+        // Captures following sequence:
+        //
+        // isinst Foo
+        // Optional:
+        //     stloc Y
+        //     ldloc Y
+        // Optional:
+        //     ldnull
+        //     cgt.un
+        //     Optional:
+        //         stloc X
+        //         ldloc X
+        // brfalse
+        private enum State : byte
+        {
+            IsInst = 1,
+
+            IsInstStLoc,
+
+            IsInstLdnull,
+            IsInstLdnullCgt,
+            IsInstLdnullCgtStLoc,
+            IsInstLdnullCgtStLocLdLoc,
+
+            Branch,
+        }
+
+        private State _state;
+        private int _token;
+
+        public readonly bool IsIsInstBranch => _state is State.Branch;
+        public int Token => _token;
+
+        public void Advance(ILOpcode opcode, in ILReader reader, MethodIL methodIL)
+        {
+            switch (_state)
+            {
+                case 0:
+                    if (opcode == ILOpcode.isinst)
+                        (_state, _token) = (State.IsInst, reader.PeekILToken());
+                    return;
+                case State.IsInst:
+                    if (opcode is ILOpcode.brfalse or ILOpcode.brfalse_s)
+                        _state = State.Branch;
+                    else if (opcode == ILOpcode.ldnull)
+                        _state = State.IsInstLdnull;
+                    else if (IsStlocLdlocSequence(opcode, reader))
+                        _state = State.IsInstStLoc;
+                    else
+                        break;
+                    return;
+                case State.IsInstLdnull:
+                    if (opcode == ILOpcode.cgt_un)
+                        _state = State.IsInstLdnullCgt;
+                    else
+                        break;
+                    return;
+                case State.IsInstLdnullCgt:
+                    if (IsStlocLdlocSequence(opcode, reader))
+                        _state = State.IsInstLdnullCgtStLoc;
+                    else
+                        break;
+                    return;
+                case State.IsInstLdnullCgtStLoc:
+                    if (opcode == ILOpcode.ldloc || opcode == ILOpcode.ldloc_s || (opcode >= ILOpcode.ldloc_0 && opcode <= ILOpcode.ldloc_3))
+                        _state = State.IsInstLdnullCgtStLocLdLoc;
+                    else
+                        throw new UnreachableException();
+                    return;
+                case State.IsInstLdnullCgtStLocLdLoc:
+                    if (opcode is ILOpcode.brfalse or ILOpcode.brfalse_s)
+                        _state = State.Branch;
+                    else
+                        break;
+                    return;
+                case State.IsInstStLoc:
+                    if (opcode == ILOpcode.ldloc || opcode == ILOpcode.ldloc_s || (opcode >= ILOpcode.ldloc_0 && opcode <= ILOpcode.ldloc_3))
+                        _state = State.IsInst;
+                    else
+                        throw new UnreachableException();
+                    return;
+                default:
+                    throw new UnreachableException();
+            }
+
+            _state = default;
+
+            static bool IsStlocLdlocSequence(ILOpcode opcode, in ILReader reader)
+            {
+                if (opcode == ILOpcode.stloc || opcode == ILOpcode.stloc_s || (opcode >= ILOpcode.stloc_0 && opcode <= ILOpcode.stloc_3))
+                {
+                    ILReader nestedReader = reader;
+                    int locIndex = opcode switch
+                    {
+                        ILOpcode.stloc => nestedReader.ReadILUInt16(),
+                        ILOpcode.stloc_s => nestedReader.ReadILByte(),
+                        _ => opcode - ILOpcode.stloc_0,
+                    };
+                    ILOpcode otherOpcode = nestedReader.ReadILOpcode();
+                    return (otherOpcode == ILOpcode.ldloc || otherOpcode == ILOpcode.ldloc_s || (otherOpcode >= ILOpcode.ldloc_0 && otherOpcode <= ILOpcode.ldloc_3))
+                        && otherOpcode switch
+                        {
+                            ILOpcode.ldloc => nestedReader.ReadILUInt16(),
+                            ILOpcode.ldloc_s => nestedReader.ReadILByte(),
+                            _ => otherOpcode - ILOpcode.ldloc_0,
+                        } == locIndex;
+                }
+                return false;
+            }
+
+            Advance(opcode, reader, methodIL);
+        }
+    }
+}

src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj

@@ -669,6 +669,7 @@
     <Compile Include="Compiler\Logging\UnconditionalSuppressMessageAttributeState.cs" />
     <Compile Include="Compiler\XmlObjectDumper.cs" />
     <Compile Include="IL\ILImporter.Scanner.cs" />
+    <Compile Include="IL\IsInstCheckPatternAnalyzer.cs" />
     <Compile Include="IL\TypeEqualityPatternAnalyzer.cs" />
     <Compile Include="IL\Stubs\PInvokeILProvider.cs" />
     <Compile Include="IL\Stubs\StartupCode\StartupCodeMainMethod.cs" />

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

@@ -20,6 +20,7 @@ public static int Run()
         TestUnusedDefaultInterfaceMethod.Run();
         TestArrayElementTypeOperations.Run();
         TestStaticVirtualMethodOptimizations.Run();
+        TestTypeIs.Run();
         TestTypeEquals.Run();
         TestTypeEqualityDeadBranchScanRemoval.Run();
         TestTypeIsEnum.Run();
@@ -333,6 +334,53 @@ public static void Run()
         }
     }
 
+
+    class TestTypeIs
+    {
+        class Never1 { }
+        class Canary1 { }
+
+        class Maybe1<T, U> { }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static object GetTheObject() => new object();
+
+        static volatile object s_sink;
+
+        public static void Run()
+        {
+#if !DEBUG
+            {
+                RunCheck(GetTheObject());
+
+                static void RunCheck(object t)
+                {
+                    if (t is Never1)
+                    {
+                        s_sink = new Canary1();
+                    }
+                }
+
+                ThrowIfPresent(typeof(TestTypeIs), nameof(Canary1));
+            }
+
+            {
+                RunCheck<object>(new Maybe1<object, string>());
+
+                [MethodImpl(MethodImplOptions.NoInlining)]
+                static void RunCheck<T>(object t)
+                {
+                    if (t is Maybe1<T, string>)
+                    {
+                        return;
+                    }
+                    throw new Exception();
+                }
+            }
+#endif
+        }
+    }
+
     class TestTypeEquals
     {
         sealed class Gen<T> { }
@@ -382,7 +430,7 @@ static void RunCheck(Type t)
                     }
                 }
 
-                ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Canary2));
+                ThrowIfPresent(typeof(TestTypeEquals), nameof(Canary2));
             }
 
             {
@@ -397,7 +445,7 @@ static void RunCheck(object o)
                     }
                 }
 
-                ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Canary3));
+                ThrowIfPresent(typeof(TestTypeEquals), nameof(Canary3));
             }
 
             {
@@ -412,7 +460,7 @@ static void RunCheck(object o)
                     }
                 }
 
-                ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Canary4));
+                ThrowIfPresent(typeof(TestTypeEquals), nameof(Canary4));
             }
 
             {