From 19cc5c98c110d14c4b084dd921abddb7ab39b3d4 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Jan 2025 00:32:53 +0100 Subject: [PATCH 1/5] Fold "x ==/!= null" on NativeAOT for non-existent classes --- src/coreclr/jit/gentree.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9154da487bd1fa..14b25d2031a153 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7644,7 +7644,9 @@ GenTreeIntCon* Compiler::gtNewIconNode(unsigned fieldOffset, FieldSeq* fieldSeq) GenTreeIntCon* Compiler::gtNewNull() { - return gtNewIconNode(0, TYP_REF); + GenTreeIntCon* tree = gtNewIconNode(0, TYP_REF); + tree->gtVNPair.SetBoth(ValueNumStore::VNForNull()); + return tree; } GenTreeIntCon* Compiler::gtNewTrue() @@ -14436,6 +14438,23 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree) return tree; } + // For "obj ==/!= null" we can fold the comparison to true/false if VM tells us that the object cannot + // be non-null. Example: "obj == null" -> true if obj's type is FooCls and FooCls is an abstract class + // without subclasses. + // + // IsTargetAbi check is not necessary here, but it improves TP for runtimes where this optimization is + // not possible anyway. + if (IsTargetAbi(CORINFO_NATIVEAOT_ABI) && (val == 0) && op->TypeIs(TYP_REF) && tree->OperIs(GT_EQ, GT_NE)) + { + bool isExact, isNonNull; + CORINFO_CLASS_HANDLE opCls = gtGetClassHandle(op, &isExact, &isNonNull); + if ((opCls != NO_CLASS_HANDLE) && (info.compCompHnd->getExactClasses(opCls, 0, nullptr) == 0)) + { + op = gtWrapWithSideEffects(NewMorphedIntConNode(tree->OperIs(GT_EQ) ? 1 : 0), op, GTF_ALL_EFFECT); + goto DONE_FOLD; + } + } + switch (oper) { case GT_LE: From 5d57eeacb473ebf6e4665297f39f2d1041cd9616 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Jan 2025 12:17:48 +0100 Subject: [PATCH 2/5] code clean up --- src/coreclr/jit/gentree.cpp | 38 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 14b25d2031a153..0ab30837412ff7 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7644,9 +7644,7 @@ GenTreeIntCon* Compiler::gtNewIconNode(unsigned fieldOffset, FieldSeq* fieldSeq) GenTreeIntCon* Compiler::gtNewNull() { - GenTreeIntCon* tree = gtNewIconNode(0, TYP_REF); - tree->gtVNPair.SetBoth(ValueNumStore::VNForNull()); - return tree; + return gtNewIconNode(0, TYP_REF); } GenTreeIntCon* Compiler::gtNewTrue() @@ -14438,23 +14436,6 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree) return tree; } - // For "obj ==/!= null" we can fold the comparison to true/false if VM tells us that the object cannot - // be non-null. Example: "obj == null" -> true if obj's type is FooCls and FooCls is an abstract class - // without subclasses. - // - // IsTargetAbi check is not necessary here, but it improves TP for runtimes where this optimization is - // not possible anyway. - if (IsTargetAbi(CORINFO_NATIVEAOT_ABI) && (val == 0) && op->TypeIs(TYP_REF) && tree->OperIs(GT_EQ, GT_NE)) - { - bool isExact, isNonNull; - CORINFO_CLASS_HANDLE opCls = gtGetClassHandle(op, &isExact, &isNonNull); - if ((opCls != NO_CLASS_HANDLE) && (info.compCompHnd->getExactClasses(opCls, 0, nullptr) == 0)) - { - op = gtWrapWithSideEffects(NewMorphedIntConNode(tree->OperIs(GT_EQ) ? 1 : 0), op, GTF_ALL_EFFECT); - goto DONE_FOLD; - } - } - switch (oper) { case GT_LE: @@ -14504,6 +14485,23 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree) case GT_EQ: case GT_NE: { + // For "obj ==/!= null" we can fold the comparison to true/false if VM tells us that the object cannot + // be non-null. Example: "obj == null" -> true if obj's type is FooCls and FooCls is an abstract class + // without subclasses. + // + // IsTargetAbi check is not necessary here, but it improves TP for runtimes where this optimization is + // not possible anyway. + if ((val == 0) && op->TypeIs(TYP_REF) && IsTargetAbi(CORINFO_NATIVEAOT_ABI)) + { + bool isExact, isNonNull; + CORINFO_CLASS_HANDLE opCls = gtGetClassHandle(op, &isExact, &isNonNull); + if ((opCls != NO_CLASS_HANDLE) && (info.compCompHnd->getExactClasses(opCls, 0, nullptr) == 0)) + { + op = gtWrapWithSideEffects(NewMorphedIntConNode(tree->OperIs(GT_EQ) ? 1 : 0), op, GTF_ALL_EFFECT); + goto DONE_FOLD; + } + } + // Optimize boxed value classes; these are always false. This IL is // generated when a generic value is tested against null: // ... foo(T x) { ... if ((object)x == null) ... From 532dfb7c4e6752d8c0b750bd62d46ecbd19c8d93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 17 Jan 2025 10:06:36 +0100 Subject: [PATCH 3/5] wip --- .../Compiler/DevirtualizationManager.cs | 15 +++ .../Compiler/Compilation.cs | 10 ++ .../ILCompiler.Compiler/Compiler/ILScanner.cs | 23 ++++ .../JitInterface/CorInfoImpl.RyuJit.cs | 17 +++ .../TrimmingBehaviors/DeadCodeElimination.cs | 100 ++++++++++++++++++ 5 files changed, 165 insertions(+) diff --git a/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs b/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs index 5574a9f8aa100f..7d1c3be1de42a9 100644 --- a/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs +++ b/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs @@ -208,6 +208,12 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType /// public virtual bool CanReferenceConstructedMethodTable(TypeDesc type) => true; + /// + /// Gets a value indicating whether it might be possible that the interface type is implemented by a constructed type data structure + /// in this compilation. Note that the MethodTable for the interface itself could still be optimized away. + /// + public virtual bool CanInterfaceBeImplementedByConstructedMethodTable(TypeDesc type) => true; + /// /// Gets a value indicating whether a (potentially canonically-equlivalent) constructed MethodTable could /// exist. This is similar to , but will return true @@ -215,6 +221,15 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType /// public virtual bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type) => true; + /// + /// Gets a value indicating whether it might be possible that the interface type is implemented by a constructed type data structure + /// in this compilation. Note that the MethodTable for the interface itself could still be optimized away. + /// Similar to but will return true for + /// IInterface<__Canon> if IInterface<String> could be implemented. + /// + public virtual bool CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(TypeDesc type) => true; + + public virtual TypeDesc[] GetImplementingClasses(TypeDesc type) => null; #endif } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index 6a542d8d00a748..4673b0f7b18421 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -108,11 +108,21 @@ public bool CanReferenceConstructedMethodTable(TypeDesc type) return NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type.NormalizeInstantiation()); } + public bool CanInterfaceBeImplementedByConstructedMethodTable(TypeDesc type) + { + return NodeFactory.DevirtualizationManager.CanInterfaceBeImplementedByConstructedMethodTable(type.NormalizeInstantiation()); + } + public bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type) { return NodeFactory.DevirtualizationManager.CanReferenceConstructedTypeOrCanonicalFormOfType(type.NormalizeInstantiation()); } + public bool CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(TypeDesc type) + { + return NodeFactory.DevirtualizationManager.CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(type.NormalizeInstantiation()); + } + public DelegateCreationInfo GetDelegateCtor(TypeDesc delegateType, MethodDesc target, TypeDesc constrainedType, bool followVirtualDispatch) { // If we're creating a delegate to a virtual method that cannot be overridden, devirtualize. diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index 4d9328f1a34eef..954ae7b92b256e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -438,6 +438,8 @@ private sealed class ScannedDevirtualizationManager : DevirtualizationManager private HashSet _constructedMethodTables = new HashSet(); private HashSet _canonConstructedMethodTables = new HashSet(); private HashSet _canonConstructedTypes = new HashSet(); + private HashSet _implementedInterfaces = new HashSet(); + private HashSet _canonImplementedInterfaces = new HashSet(); private HashSet _unsealedTypes = new HashSet(); private Dictionary> _implementators = new(); private HashSet _disqualifiedTypes = new(); @@ -507,6 +509,13 @@ public ScannedDevirtualizationManager(NodeFactory factory, ImmutableArray; + interface INever3; + interface INever3_Variant; + + class ImplementsINever3Object : INever3; + class ImplementsINever3_VariantObject : INever3_Variant; + + interface IImplemented; + class Implements : IImplemented; + + class Canary1; + class Canary2; + class Canary3; + class Canary4; + class Canary5; + + [MethodImpl(MethodImplOptions.NoInlining)] + static object AcceptINever(INever1 inst) + { + if (inst != null) + { + return new Canary1(); + } + return null; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static object AcceptINever2(INever2 inst) + { + if (inst != null) + { + return new Canary2(); + } + return null; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static object AcceptIImplemented(IImplemented inst) + { + if (inst != null) + { + return new Canary3(); + } + return null; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static object AcceptINever3String(INever3 inst) + { + if (inst != null) + { + return new Canary4(); + } + return null; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static object AcceptINever3_VariantString(INever3_Variant inst) + { + if (inst != null) + { + return new Canary5(); + } + return null; + } + + public static void Run() + { + if (AcceptINever(null) != null) + throw new Exception(); +#if !DEBUG + ThrowIfPresentWithUsableMethodTable(typeof(TestCanBeNonNull), nameof(Canary1)); +#endif + + if (AcceptINever2(null) != null) + throw new Exception(); +#if !DEBUG + ThrowIfPresentWithUsableMethodTable(typeof(TestCanBeNonNull), nameof(Canary2)); +#endif + + if (AcceptIImplemented(new Implements()) == null) + throw new Exception(); + ThrowIfNotPresent(typeof(TestCanBeNonNull), nameof(Canary3)); + + new ImplementsINever3Object().ToString(); + AcceptINever3String(null); +#if !DEBUG + ThrowIfPresentWithUsableMethodTable(typeof(TestCanBeNonNull), nameof(Canary4)); +#endif + + if (AcceptINever3_VariantString(new ImplementsINever3_VariantObject()) == null) + throw new Exception(); + ThrowIfNotPresent(typeof(TestCanBeNonNull), nameof(Canary5)); + } + } + class TestUnmodifiableStaticFieldOptimization { static class ClassWithNotReadOnlyField From 0ce09bc2af624c05284e17be7ed1c6d8423a79ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 17 Jan 2025 10:45:28 +0100 Subject: [PATCH 4/5] What if we break IDynIntfCastable --- .../ILCompiler.Compiler/Compiler/Compilation.cs | 10 +++++----- .../InterfaceDispatchCellNode.cs | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index 4673b0f7b18421..95b87d1ec32136 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -312,11 +312,11 @@ public ISymbolNode ComputeConstantLookup(ReadyToRunHelperId lookupKind, object t { var type = (TypeDesc)targetOfLookup; - // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. - // If this cast happens with an object that implements IDynamicIntefaceCastable, user code will - // see a RuntimeTypeHandle representing this interface. - if (type.IsInterface) - return NodeFactory.MaximallyConstructableType(type); + //// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. + //// If this cast happens with an object that implements IDynamicIntefaceCastable, user code will + //// see a RuntimeTypeHandle representing this interface. + //if (type.IsInterface) + // return NodeFactory.MaximallyConstructableType(type); if (type.IsNullable) type = type.Instantiation[0]; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs index e0bf93690edf8f..3152330bbc80ff 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs @@ -63,10 +63,10 @@ public override IEnumerable GetStaticDependencies(NodeFacto result.Add(factory.InitialInterfaceDispatchStub, "Initial interface dispatch stub"); - // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. - // If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will - // see a RuntimeTypeHandle representing this interface. - result.Add(factory.ConstructedTypeSymbol(_targetMethod.OwningType), "Interface type"); + //// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. + //// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will + //// see a RuntimeTypeHandle representing this interface. + result.Add(factory.NecessaryTypeSymbol(_targetMethod.OwningType), "Interface type"); return result; } @@ -75,10 +75,10 @@ public override void EncodeData(ref ObjectDataBuilder objData, NodeFactory facto { objData.EmitPointerReloc(factory.InitialInterfaceDispatchStub); - // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. - // If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will - // see a RuntimeTypeHandle representing this interface. - IEETypeNode interfaceType = factory.ConstructedTypeSymbol(_targetMethod.OwningType); + //// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. + //// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will + //// see a RuntimeTypeHandle representing this interface. + IEETypeNode interfaceType = factory.NecessaryTypeSymbol(_targetMethod.OwningType); if (factory.Target.SupportsRelativePointers) { if (interfaceType.RepresentsIndirectionCell) From 4cfa2e6cde639583bc1bf457bec9c79477471bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Mon, 20 Jan 2025 09:06:22 +0100 Subject: [PATCH 5/5] Revert "What if we break IDynIntfCastable" This reverts commit 0ce09bc2af624c05284e17be7ed1c6d8423a79ea. --- .../ILCompiler.Compiler/Compiler/Compilation.cs | 10 +++++----- .../InterfaceDispatchCellNode.cs | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index 95b87d1ec32136..4673b0f7b18421 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -312,11 +312,11 @@ public ISymbolNode ComputeConstantLookup(ReadyToRunHelperId lookupKind, object t { var type = (TypeDesc)targetOfLookup; - //// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. - //// If this cast happens with an object that implements IDynamicIntefaceCastable, user code will - //// see a RuntimeTypeHandle representing this interface. - //if (type.IsInterface) - // return NodeFactory.MaximallyConstructableType(type); + // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. + // If this cast happens with an object that implements IDynamicIntefaceCastable, user code will + // see a RuntimeTypeHandle representing this interface. + if (type.IsInterface) + return NodeFactory.MaximallyConstructableType(type); if (type.IsNullable) type = type.Instantiation[0]; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs index 3152330bbc80ff..e0bf93690edf8f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs @@ -63,10 +63,10 @@ public override IEnumerable GetStaticDependencies(NodeFacto result.Add(factory.InitialInterfaceDispatchStub, "Initial interface dispatch stub"); - //// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. - //// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will - //// see a RuntimeTypeHandle representing this interface. - result.Add(factory.NecessaryTypeSymbol(_targetMethod.OwningType), "Interface type"); + // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. + // If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will + // see a RuntimeTypeHandle representing this interface. + result.Add(factory.ConstructedTypeSymbol(_targetMethod.OwningType), "Interface type"); return result; } @@ -75,10 +75,10 @@ public override void EncodeData(ref ObjectDataBuilder objData, NodeFactory facto { objData.EmitPointerReloc(factory.InitialInterfaceDispatchStub); - //// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. - //// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will - //// see a RuntimeTypeHandle representing this interface. - IEETypeNode interfaceType = factory.NecessaryTypeSymbol(_targetMethod.OwningType); + // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. + // If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will + // see a RuntimeTypeHandle representing this interface. + IEETypeNode interfaceType = factory.ConstructedTypeSymbol(_targetMethod.OwningType); if (factory.Target.SupportsRelativePointers) { if (interfaceType.RepresentsIndirectionCell)