Skip to content

Commit 9c7ee97

Browse files
Avoid scanning typeof checks when building whole program view (#103883)
Before this PR, we were somewhat able to eliminate dead typeof checks such as: ```csharp if (someType == typeof(Foo) { ExpensiveMethod(); } ``` This work was done in #102248. 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. 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.
1 parent fee1b41 commit 9c7ee97

File tree

6 files changed

+184
-26
lines changed

6 files changed

+184
-26
lines changed

src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowConcreteMethodNode.cs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,37 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
8585
return dependencies;
8686
}
8787

88+
public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
89+
{
90+
// Instantiate the runtime determined dependencies of the canonical method body
91+
// with the concrete instantiation of the method to get concrete dependencies.
92+
Instantiation typeInst = Method.OwningType.Instantiation;
93+
Instantiation methodInst = Method.Instantiation;
94+
IEnumerable<CombinedDependencyListEntry> staticDependencies = CanonicalMethodNode.GetConditionalStaticDependencies(factory);
95+
96+
if (staticDependencies != null)
97+
{
98+
foreach (CombinedDependencyListEntry canonDep in staticDependencies)
99+
{
100+
Debug.Assert(canonDep.OtherReasonNode is not INodeWithRuntimeDeterminedDependencies);
101+
102+
var node = canonDep.Node;
103+
if (node is INodeWithRuntimeDeterminedDependencies runtimeDeterminedNode)
104+
{
105+
foreach (var nodeInner in runtimeDeterminedNode.InstantiateDependencies(factory, typeInst, methodInst))
106+
yield return new CombinedDependencyListEntry(nodeInner.Node, canonDep.OtherReasonNode, nodeInner.Reason);
107+
}
108+
}
109+
}
110+
}
111+
88112
protected override string GetName(NodeFactory factory) => $"{Method} backed by {CanonicalMethodNode.GetMangledName(factory.NameMangler)}";
89113

90-
public sealed override bool HasConditionalStaticDependencies => false;
114+
public sealed override bool HasConditionalStaticDependencies => CanonicalMethodNode.HasConditionalStaticDependencies;
91115
public sealed override bool HasDynamicDependencies => false;
92116
public sealed override bool InterestingForDynamicDependencyAnalysis => false;
93117

94118
public sealed override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null;
95-
public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
96119

97120
int ISortableNode.ClassCode => -1440570971;
98121

src/coreclr/tools/Common/TypeSystem/IL/ILImporter.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,12 +301,17 @@ private void ImportBasicBlocks()
301301
}
302302

303303
private void MarkBasicBlock(BasicBlock basicBlock)
304+
{
305+
MarkBasicBlock(basicBlock, ref _pendingBasicBlocks);
306+
}
307+
308+
private static void MarkBasicBlock(BasicBlock basicBlock, ref BasicBlock list)
304309
{
305310
if (basicBlock.State == BasicBlock.ImportState.Unmarked)
306311
{
307312
// Link
308-
basicBlock.Next = _pendingBasicBlocks;
309-
_pendingBasicBlocks = basicBlock;
313+
basicBlock.Next = list;
314+
list = basicBlock;
310315

311316
basicBlock.State = BasicBlock.ImportState.IsPending;
312317
}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ScannedMethodNode.cs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public class ScannedMethodNode : DependencyNodeCore<NodeFactory>, IMethodBodyNod
2222
{
2323
private readonly MethodDesc _method;
2424
private DependencyList _dependencies;
25+
private CombinedDependencyList _conditionalDependencies;
2526

2627
// If we failed to scan the method, the dependencies reported by the node will
2728
// be for a throwing method body. This field will store the underlying cause of the failure.
@@ -42,11 +43,14 @@ public ScannedMethodNode(MethodDesc method)
4243

4344
public bool RepresentsIndirectionCell => false;
4445

46+
public override bool HasConditionalStaticDependencies => _conditionalDependencies != null;
47+
4548
public override bool StaticDependenciesAreComputed => _dependencies != null;
4649

47-
public void InitializeDependencies(NodeFactory factory, IEnumerable<DependencyListEntry> dependencies, TypeSystemException scanningException = null)
50+
public void InitializeDependencies(NodeFactory factory, (DependencyList, CombinedDependencyList) dependencies, TypeSystemException scanningException = null)
4851
{
49-
_dependencies = new DependencyList(dependencies);
52+
_dependencies = dependencies.Item1;
53+
_conditionalDependencies = dependencies.Item2;
5054

5155
if (factory.TypeSystemContext.IsSpecialUnboxingThunk(_method))
5256
{
@@ -72,19 +76,13 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
7276
return _dependencies;
7377
}
7478

75-
protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler);
79+
public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => _conditionalDependencies;
7680

77-
public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
78-
{
79-
CombinedDependencyList dependencies = null;
80-
CodeBasedDependencyAlgorithm.AddConditionalDependenciesDueToMethodCodePresence(ref dependencies, factory, _method);
81-
return dependencies ?? (IEnumerable<CombinedDependencyListEntry>)Array.Empty<CombinedDependencyListEntry>();
82-
}
81+
protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler);
8382

8483
public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null;
8584
public override bool InterestingForDynamicDependencyAnalysis => _method.HasInstantiation || _method.OwningType.HasInstantiation;
8685
public override bool HasDynamicDependencies => false;
87-
public override bool HasConditionalStaticDependencies => CodeBasedDependencyAlgorithm.HasConditionalDependenciesDueToMethodCodePresence(_method);
8886

8987
int ISortableNode.ClassCode => -1381809560;
9088

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

Lines changed: 105 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
using Debug = System.Diagnostics.Debug;
1111
using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.DependencyList;
12+
using CombinedDependencyList = System.Collections.Generic.List<ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.CombinedDependencyListEntry>;
13+
using DependencyListEntry = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.DependencyListEntry;
1214

1315
#pragma warning disable IDE0060
1416

@@ -28,7 +30,7 @@ internal partial class ILImporter
2830

2931
private readonly MethodDesc _canonMethod;
3032

31-
private DependencyList _dependencies = new DependencyList();
33+
private DependencyList _unconditionalDependencies = new DependencyList();
3234

3335
private readonly byte[] _ilBytes;
3436

@@ -51,11 +53,17 @@ public enum ImportState : byte
5153
public bool TryStart;
5254
public bool FilterStart;
5355
public bool HandlerStart;
56+
57+
public object Condition;
58+
public DependencyList Dependencies;
5459
}
5560

5661
private bool _isReadOnly;
5762
private TypeDesc _constrained;
5863

64+
private DependencyList _dependencies;
65+
private BasicBlock _lateBasicBlocks;
66+
5967
private sealed class ExceptionRegion
6068
{
6169
public ILExceptionRegion ILRegion;
@@ -107,9 +115,11 @@ public ILImporter(ILScanner compilation, MethodDesc method, MethodIL methodIL =
107115
{
108116
_exceptionRegions[i] = new ExceptionRegion() { ILRegion = ilExceptionRegions[i] };
109117
}
118+
119+
_dependencies = _unconditionalDependencies;
110120
}
111121

112-
public DependencyList Import()
122+
public (DependencyList, CombinedDependencyList) Import()
113123
{
114124
TypeDesc owningType = _canonMethod.OwningType;
115125
if (_compilation.HasLazyStaticConstructor(owningType))
@@ -172,9 +182,21 @@ public DependencyList Import()
172182
FindBasicBlocks();
173183
ImportBasicBlocks();
174184

175-
CodeBasedDependencyAlgorithm.AddDependenciesDueToMethodCodePresence(ref _dependencies, _factory, _canonMethod, _canonMethodIL);
185+
CombinedDependencyList conditionalDependencies = null;
186+
foreach (BasicBlock bb in _basicBlocks)
187+
{
188+
if (bb?.Condition == null)
189+
continue;
190+
191+
conditionalDependencies ??= new CombinedDependencyList();
192+
foreach (DependencyListEntry dep in bb.Dependencies)
193+
conditionalDependencies.Add(new(dep.Node, bb.Condition, dep.Reason));
194+
}
195+
196+
CodeBasedDependencyAlgorithm.AddDependenciesDueToMethodCodePresence(ref _unconditionalDependencies, _factory, _canonMethod, _canonMethodIL);
197+
CodeBasedDependencyAlgorithm.AddConditionalDependenciesDueToMethodCodePresence(ref conditionalDependencies, _factory, _canonMethod);
176198

177-
return _dependencies;
199+
return (_unconditionalDependencies, conditionalDependencies);
178200
}
179201

180202
private ISymbolNode GetGenericLookupHelper(ReadyToRunHelperId helperId, object helperArgument)
@@ -199,19 +221,29 @@ private ISymbolNode GetHelperEntrypoint(ReadyToRunHelper helper)
199221
}
200222

201223
private static void MarkInstructionBoundary() { }
202-
private static void EndImportingBasicBlock(BasicBlock basicBlock) { }
224+
225+
private void EndImportingBasicBlock(BasicBlock basicBlock)
226+
{
227+
if (_pendingBasicBlocks == null)
228+
{
229+
_pendingBasicBlocks = _lateBasicBlocks;
230+
_lateBasicBlocks = null;
231+
}
232+
}
203233

204234
private void StartImportingBasicBlock(BasicBlock basicBlock)
205235
{
236+
_dependencies = basicBlock.Condition != null ? basicBlock.Dependencies : _unconditionalDependencies;
237+
206238
// Import all associated EH regions
207239
foreach (ExceptionRegion ehRegion in _exceptionRegions)
208240
{
209241
ILExceptionRegion region = ehRegion.ILRegion;
210242
if (region.TryOffset == basicBlock.StartOffset)
211243
{
212-
MarkBasicBlock(_basicBlocks[region.HandlerOffset]);
244+
ImportBasicBlockEdge(basicBlock, _basicBlocks[region.HandlerOffset]);
213245
if (region.Kind == ILExceptionRegionKind.Filter)
214-
MarkBasicBlock(_basicBlocks[region.FilterOffset]);
246+
ImportBasicBlockEdge(basicBlock, _basicBlocks[region.FilterOffset]);
215247

216248
if (region.Kind == ILExceptionRegionKind.Catch)
217249
{
@@ -789,10 +821,26 @@ private void ImportCalli(int token)
789821

790822
private void ImportBranch(ILOpcode opcode, BasicBlock target, BasicBlock fallthrough)
791823
{
824+
object condition = null;
825+
826+
if (opcode == ILOpcode.brfalse
827+
&& _typeEqualityPatternAnalyzer.IsTypeEqualityBranch
828+
&& !_typeEqualityPatternAnalyzer.IsTwoTokens
829+
&& !_typeEqualityPatternAnalyzer.IsInequality)
830+
{
831+
TypeDesc typeEqualityCheckType = (TypeDesc)_canonMethodIL.GetObject(_typeEqualityPatternAnalyzer.Token1);
832+
if (!typeEqualityCheckType.IsGenericDefinition
833+
&& ConstructedEETypeNode.CreationAllowed(typeEqualityCheckType)
834+
&& !typeEqualityCheckType.ConvertToCanonForm(CanonicalFormKind.Specific).IsCanonicalSubtype(CanonicalFormKind.Any))
835+
{
836+
condition = _factory.MaximallyConstructableType(typeEqualityCheckType);
837+
}
838+
}
839+
792840
ImportFallthrough(target);
793841

794842
if (fallthrough != null)
795-
ImportFallthrough(fallthrough);
843+
ImportFallthrough(fallthrough, condition);
796844
}
797845

798846
private void ImportSwitchJump(int jmpBase, int[] jmpDelta, BasicBlock fallthrough)
@@ -1278,9 +1326,56 @@ private void ImportConvert(WellKnownType wellKnownType, bool checkOverflow, bool
12781326
}
12791327
}
12801328

1281-
private void ImportFallthrough(BasicBlock next)
1329+
private void ImportBasicBlockEdge(BasicBlock source, BasicBlock next, object condition = null)
1330+
{
1331+
// We don't track multiple conditions; if the source basic block is only reachable under a condition,
1332+
// this condition will be used for the next basic block, irrespective if we could make it more narrow.
1333+
object effectiveCondition = source.Condition ?? condition;
1334+
1335+
// Did we already look at this basic block?
1336+
if (next.State != BasicBlock.ImportState.Unmarked)
1337+
{
1338+
// If next is not conditioned, it stays not conditioned.
1339+
// If it was conditioned on something else, it needs to become unconditional.
1340+
// If the conditions match, it stays conditioned on the same thing.
1341+
if (next.Condition != null && next.Condition != effectiveCondition)
1342+
{
1343+
// Now we need to make `next` not conditioned. We move all of its dependencies to
1344+
// unconditional dependencies, and do this for all basic blocks that are reachable
1345+
// from it.
1346+
// TODO-SIZE: below doesn't do it for all basic blocks reachable from `next`, but
1347+
// for all basic blocks with the same conditon. This is a shortcut. It likely
1348+
// doesn't matter in practice.
1349+
object conditionToRemove = next.Condition;
1350+
foreach (BasicBlock bb in _basicBlocks)
1351+
{
1352+
if (bb?.Condition == conditionToRemove)
1353+
{
1354+
_unconditionalDependencies.AddRange(bb.Dependencies);
1355+
bb.Dependencies = null;
1356+
bb.Condition = null;
1357+
}
1358+
}
1359+
}
1360+
}
1361+
else
1362+
{
1363+
if (effectiveCondition != null)
1364+
{
1365+
next.Condition = effectiveCondition;
1366+
next.Dependencies = new DependencyList();
1367+
MarkBasicBlock(next, ref _lateBasicBlocks);
1368+
}
1369+
else
1370+
{
1371+
MarkBasicBlock(next);
1372+
}
1373+
}
1374+
}
1375+
1376+
private void ImportFallthrough(BasicBlock next, object condition = null)
12821377
{
1283-
MarkBasicBlock(next);
1378+
ImportBasicBlockEdge(_currentBasicBlock, next, condition);
12841379
}
12851380

12861381
private int ReadILTokenAt(int ilOffset)

src/tests/nativeaot/SmokeTests/HardwareIntrinsics/Program.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ static int Main()
2121
Console.WriteLine("****************************************************");
2222

2323
long lowerBound, upperBound;
24-
lowerBound = 1300 * 1024; // ~1.3 MB
25-
upperBound = 1900 * 1024; // ~1.90 MB
24+
lowerBound = 1200 * 1024; // ~1.2 MB
25+
upperBound = 1600 * 1024; // ~1.6 MB
2626

2727
if (fileSize < lowerBound || fileSize > upperBound)
2828
{

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public static int Run()
2121
TestArrayElementTypeOperations.Run();
2222
TestStaticVirtualMethodOptimizations.Run();
2323
TestTypeEquals.Run();
24+
TestTypeEqualityDeadBranchScanRemoval.Run();
2425
TestTypeIsEnum.Run();
2526
TestTypeIsValueType.Run();
2627
TestBranchesInGenericCodeRemoval.Run();
@@ -452,6 +453,42 @@ static void RunCheck<T>(Type t)
452453
}
453454
}
454455

456+
class TestTypeEqualityDeadBranchScanRemoval
457+
{
458+
class NeverAllocated1 { }
459+
class NeverAllocated2 { }
460+
461+
class PossiblyAllocated1 { }
462+
class PossiblyAllocated2 { }
463+
464+
[MethodImpl(MethodImplOptions.NoInlining)]
465+
static Type GetNeverObject() => null;
466+
467+
static volatile Type s_sink;
468+
469+
public static void Run()
470+
{
471+
if (GetNeverObject() == typeof(NeverAllocated1))
472+
{
473+
Console.WriteLine(new NeverAllocated1().ToString());
474+
Console.WriteLine(new NeverAllocated2().ToString());
475+
}
476+
#if !DEBUG
477+
ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEqualityDeadBranchScanRemoval), nameof(NeverAllocated1));
478+
ThrowIfPresent(typeof(TestTypeEqualityDeadBranchScanRemoval), nameof(NeverAllocated2));
479+
#endif
480+
481+
if (GetNeverObject() == typeof(PossiblyAllocated1))
482+
{
483+
Console.WriteLine(new PossiblyAllocated1().ToString());
484+
Console.WriteLine(new PossiblyAllocated2().ToString());
485+
}
486+
if (Environment.GetEnvironmentVariable("SURETHING") != null)
487+
s_sink = typeof(PossiblyAllocated1);
488+
}
489+
}
490+
491+
455492
class TestTypeIsEnum
456493
{
457494
class Never { }

0 commit comments

Comments
 (0)