Skip to content

Commit

Permalink
Fold always false type checks (#99761)
Browse files Browse the repository at this point in the history
If we have a program like:

```csharp
class Never { }

class Program
{
    static void Main()
    {
        Test(null);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Test(object o)
    {
        if (o is Never)
            Console.WriteLine("Hello!");
        if (o.GetType() == typeof(Never))
            Console.WriteLine("Hello!");
    }
}
```

We know these checks are never going to be true thanks to the whole program view. Fold these to `false`.
  • Loading branch information
MichalStrehovsky authored Mar 18, 2024
1 parent 186c994 commit 21cfd9f
Show file tree
Hide file tree
Showing 15 changed files with 241 additions and 32 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -2983,7 +2983,7 @@ class ICorStaticInfo
CORINFO_CLASS_HANDLE* vcTypeRet /* OUT */
) = 0;

// Obtains a list of exact classes for a given base type. Returns 0 if the number of
// Obtains a list of exact classes for a given base type. Returns -1 if the number of
// the exact classes is greater than maxExactClasses or if more types might be loaded
// in future.
virtual int getExactClasses(
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* 6fd660c7-96be-4832-a84c-4200141f7d08 */
0x6fd660c7,
0x96be,
0x4832,
{0xa8, 0x4c, 0x42, 0x00, 0x14, 0x1f, 0x7d, 0x08}
constexpr GUID JITEEVersionIdentifier = { /* bdf34b26-0725-4ad6-9935-40bfd2a4c4fc */
0xbdf34b26,
0x0725,
0x4ad6,
{0x99, 0x35, 0x40, 0xbf, 0xd2, 0xa4, 0xc4, 0xfc}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14117,6 +14117,18 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree)
return tree;
}

// Check if an object of this type can even exist
if (info.compCompHnd->getExactClasses(clsHnd, 0, nullptr) == 0)
{
JITDUMP("Runtime reported %p (%s) is never allocated\n", dspPtr(clsHnd), eeGetClassName(clsHnd));

const bool operatorIsEQ = (oper == GT_EQ);
const int compareResult = operatorIsEQ ? 0 : 1;
JITDUMP("Runtime reports comparison is known at jit time: %u\n", compareResult);
GenTree* result = gtNewIconNode(compareResult);
return result;
}

// We're good to go.
JITDUMP("Optimizing compare of obj.GetType()"
" and type-from-handle to compare method table pointer\n");
Expand Down
27 changes: 26 additions & 1 deletion src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5380,14 +5380,39 @@ GenTree* Compiler::impOptimizeCastClassOrIsInst(GenTree* op1, CORINFO_RESOLVED_T
return nullptr;
}

CORINFO_CLASS_HANDLE toClass = pResolvedToken->hClass;
if (info.compCompHnd->getExactClasses(toClass, 0, nullptr) == 0)
{
JITDUMP("\nClass %p (%s) can never be allocated\n", dspPtr(toClass), eeGetClassName(toClass));

if (!isCastClass)
{
JITDUMP("Cast will fail, optimizing to return null\n");

// If the cast was fed by a box, we can remove that too.
if (op1->IsBoxedValue())
{
JITDUMP("Also removing upstream box\n");
gtTryRemoveBoxUpstreamEffects(op1);
}

if (gtTreeHasSideEffects(op1, GTF_SIDE_EFFECT))
{
impAppendTree(op1, CHECK_SPILL_ALL, impCurStmtDI);
}
return gtNewNull();
}

JITDUMP("Cast will always throw, but not optimizing yet\n");
}

// See what we know about the type of the object being cast.
bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE fromClass = gtGetClassHandle(op1, &isExact, &isNonNull);

if (fromClass != nullptr)
{
CORINFO_CLASS_HANDLE toClass = pResolvedToken->hClass;
JITDUMP("\nConsidering optimization of %s from %s%p (%s) to %p (%s)\n", isCastClass ? "castclass" : "isinst",
isExact ? "exact " : "", dspPtr(fromClass), eeGetClassName(fromClass), dspPtr(toClass),
eeGetClassName(toClass));
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6596,7 +6596,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
{
JITDUMP("No exact classes implementing %s\n", eeGetClassName(baseClass))
}
else if (numExactClasses > maxTypeChecks)
else if (numExactClasses < 0 || numExactClasses > maxTypeChecks)
{
JITDUMP("Too many exact classes implementing %s (%d > %d)\n", eeGetClassName(baseClass), numExactClasses,
maxTypeChecks)
Expand Down
16 changes: 10 additions & 6 deletions src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,17 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType

#if !READYTORUN
/// <summary>
/// Gets a value indicating whether it might be possible to obtain a constructed type data structure for the given type.
/// Gets a value indicating whether it might be possible to obtain a constructed type data structure for the given type
/// in this compilation (i.e. is it possible to reference a constructed MethodTable symbol for this).
/// </summary>
/// <remarks>
/// This is a bit of a hack, but devirtualization manager has a global view of all allocated types
/// so it can answer this question.
/// </remarks>
public virtual bool CanConstructType(TypeDesc type) => true;
public virtual bool CanReferenceConstructedMethodTable(TypeDesc type) => true;

/// <summary>
/// Gets a value indicating whether a (potentially canonically-equlivalent) constructed MethodTable could
/// exist. This is similar to <see cref="CanReferenceConstructedMethodTable"/>, but will return true
/// for List&lt;__Canon&gt; if a constructed MethodTable for List&lt;object&gt; exists.
/// </summary>
public virtual bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type) => true;

public virtual TypeDesc[] GetImplementingClasses(TypeDesc type) => null;
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4284,7 +4284,7 @@ private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref
#pragma warning disable SA1001, SA1113, SA1115 // Commas should be spaced correctly
ComputeJitPgoInstrumentationSchema(ObjectToHandle, pgoResultsSchemas, out var nativeSchemas, _cachedMemoryStream
#if !READYTORUN
, _compilation.CanConstructType
, _compilation.CanReferenceConstructedMethodTable
#endif
);
#pragma warning restore SA1001, SA1113, SA1115 // Commas should be spaced correctly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,14 @@ public bool CanInline(MethodDesc caller, MethodDesc callee)
return _inliningPolicy.CanInline(caller, callee);
}

public bool CanConstructType(TypeDesc type)
public bool CanReferenceConstructedMethodTable(TypeDesc type)
{
return NodeFactory.DevirtualizationManager.CanConstructType(type);
return NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type);
}

public bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type)
{
return NodeFactory.DevirtualizationManager.CanReferenceConstructedTypeOrCanonicalFormOfType(type);
}

public DelegateCreationInfo GetDelegateCtor(TypeDesc delegateType, MethodDesc target, TypeDesc constrainedType, bool followVirtualDispatch)
Expand Down Expand Up @@ -261,7 +266,7 @@ public bool NeedsRuntimeLookup(ReadyToRunHelperId lookupKind, object targetOfLoo

public ReadyToRunHelperId GetLdTokenHelperForType(TypeDesc type)
{
bool canConstructPerWholeProgramAnalysis = NodeFactory.DevirtualizationManager.CanConstructType(type);
bool canConstructPerWholeProgramAnalysis = NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type);
bool creationAllowed = ConstructedEETypeNode.CreationAllowed(type);
return (canConstructPerWholeProgramAnalysis && creationAllowed)
? ReadyToRunHelperId.TypeHandle
Expand Down
16 changes: 13 additions & 3 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,8 @@ public override DictionaryLayoutNode GetLayout(TypeSystemEntity methodOrType)

private sealed class ScannedDevirtualizationManager : DevirtualizationManager
{
private HashSet<TypeDesc> _constructedTypes = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _constructedMethodTables = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _canonConstructedMethodTables = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _canonConstructedTypes = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _unsealedTypes = new HashSet<TypeDesc>();
private Dictionary<TypeDesc, HashSet<TypeDesc>> _implementators = new();
Expand Down Expand Up @@ -442,7 +443,12 @@ public ScannedDevirtualizationManager(NodeFactory factory, ImmutableArray<Depend

if (type != null)
{
_constructedTypes.Add(type);
_constructedMethodTables.Add(type);
TypeDesc canonForm = type.ConvertToCanonForm(CanonicalFormKind.Specific);
if (canonForm != type)
{
_canonConstructedMethodTables.Add(canonForm);
}

if (type.IsInterface)
{
Expand Down Expand Up @@ -687,7 +693,11 @@ protected override MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefTyp
return result;
}

public override bool CanConstructType(TypeDesc type) => _constructedTypes.Contains(type);
public override bool CanReferenceConstructedMethodTable(TypeDesc type)
=> _constructedMethodTables.Contains(type);

public override bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type)
=> _constructedMethodTables.Contains(type) || _canonConstructedMethodTables.Contains(type);

public override TypeDesc[] GetImplementingClasses(TypeDesc type)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3280,7 +3280,7 @@ private void updateEntryPointForTailCall(ref CORINFO_CONST_LOOKUP entryPoint)
private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses, CORINFO_CLASS_STRUCT_** exactClsRet)
{
// Not implemented for R2R yet
return 0;
return -1;
}

private bool getStaticFieldContent(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type)
// information proving that it isn't, give RyuJIT the constructed symbol even
// though we just need the unconstructed one.
// https://github.com/dotnet/runtimelab/issues/1128
bool canPotentiallyConstruct = NodeFactory.DevirtualizationManager.CanConstructType(type);
bool canPotentiallyConstruct = NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type);
if (canPotentiallyConstruct)
return _nodeFactory.MaximallyConstructableType(type);

Expand All @@ -81,7 +81,7 @@ public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type)

public FrozenRuntimeTypeNode NecessaryRuntimeTypeIfPossible(TypeDesc type)
{
bool canPotentiallyConstruct = NodeFactory.DevirtualizationManager.CanConstructType(type);
bool canPotentiallyConstruct = NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type);
if (canPotentiallyConstruct)
return _nodeFactory.SerializedMaximallyConstructableRuntimeTypeObject(type);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2248,14 +2248,42 @@ private void getFieldInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_MET
// and STS::AccessCheck::CanAccess.
}

private bool CanNeverHaveInstanceOfSubclassOf(TypeDesc type)
{
// Don't try to optimize nullable
if (type.IsNullable)
return false;

// We don't track unconstructable types very well and they are rare anyway
if (!ConstructedEETypeNode.CreationAllowed(type))
return false;

TypeDesc canonType = type.ConvertToCanonForm(CanonicalFormKind.Specific);

// If we don't have a constructed MethodTable for the exact type or for its template,
// this type or any of its subclasses can never be instantiated.
return !_compilation.CanReferenceConstructedTypeOrCanonicalFormOfType(type)
&& (type == canonType || !_compilation.CanReferenceConstructedMethodTable(canonType));
}

private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses, CORINFO_CLASS_STRUCT_** exactClsRet)
{
MetadataType type = HandleToObject(baseType) as MetadataType;
if (type == null)
{
return -1;
}

if (CanNeverHaveInstanceOfSubclassOf(type))
{
return 0;
}

if (maxExactClasses == 0)
{
return -1;
}

// type is already sealed, return it
if (_compilation.IsEffectivelySealed(type))
{
Expand All @@ -2266,7 +2294,7 @@ private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses
TypeDesc[] implClasses = _compilation.GetImplementingClasses(type);
if (implClasses == null || implClasses.Length > maxExactClasses)
{
return 0;
return -1;
}

int index = 0;
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2787,15 +2787,15 @@ void MethodContext::recGetExactClasses(CORINFO_CLASS_HANDLE baseType, int maxExa
key.A = CastHandle(baseType);
key.B = maxExactClasses;

Assert(result >= 0);
int numResults = result < 0 ? 0 : result;

DWORDLONG* exactClassesAgnostic = new DWORDLONG[result];
for (int i = 0; i < result; i++)
DWORDLONG* exactClassesAgnostic = new DWORDLONG[numResults];
for (int i = 0; i < numResults; i++)
exactClassesAgnostic[i] = CastHandle(exactClsRet[i]);

Agnostic_GetExactClassesResult value;
value.numClasses = result;
value.classes = GetExactClasses->AddBuffer((unsigned char*)exactClassesAgnostic, (unsigned int)(result * sizeof(DWORDLONG)));
value.classes = GetExactClasses->AddBuffer((unsigned char*)exactClassesAgnostic, (unsigned int)(numResults * sizeof(DWORDLONG)));

delete[] exactClassesAgnostic;

Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9663,16 +9663,14 @@ int CEEInfo::getExactClasses (
MODE_ANY;
} CONTRACTL_END;

int exactClassesCount = 0;

JIT_TO_EE_TRANSITION();

// This function is currently implemented only on NativeAOT
// but can be implemented for CoreCLR as well (e.g. for internal types)

EE_TO_JIT_TRANSITION();

return exactClassesCount;
return -1;
}

/*********************************************************************/
Expand Down
Loading

0 comments on commit 21cfd9f

Please sign in to comment.