Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Late cast expansion: remove the null check if possible #97234

Merged
merged 13 commits into from
Feb 7, 2024
14 changes: 10 additions & 4 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5004,10 +5004,16 @@ GenTree* Compiler::optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCal
return optAssertionProp_Update(arg1, call, stmt);
}

// TODO-InlineCast: check optAssertionIsNonNull for the object argument and replace
// the helper with its nonnull version, e.g.:
// CORINFO_HELP_ISINSTANCEOFANY -> CORINFO_HELP_ISINSTANCEOFANY_NONNULL
// so then fgLateCastExpansion can skip the null check.
// Leave a hint for fgLateCastExpansion that obj is never null.
INDEBUG(AssertionIndex nonNullIdx = NO_ASSERTION_INDEX);
INDEBUG(bool vnBased = false);
// GTF_CALL_M_CAST_CAN_BE_EXPANDED check is to improve TP
if (((call->gtCallMoreFlags & GTF_CALL_M_CAST_CAN_BE_EXPANDED) != 0) &&
optAssertionIsNonNull(arg1, assertions DEBUGARG(&vnBased) DEBUGARG(&nonNullIdx)))
{
call->gtCallMoreFlags |= GTF_CALL_M_CAST_OBJ_NONNULL;
return optAssertionProp_Update(call, call, stmt);
}
}
}

Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10028,11 +10028,6 @@ JITDBGAPI void __cdecl cTreeFlags(Compiler* comp, GenTree* tree)
chars += printf("[CALL_GUARDED]");
}

if (call->IsExpRuntimeLookup())
{
chars += printf("[CALL_EXP_RUNTIME_LOOKUP]");
}

if (call->gtCallDebugFlags & GTF_CALL_MD_STRESS_TAILCALL)
{
chars += printf("[CALL_MD_STRESS_TAILCALL]");
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gcinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTreeStoreInd* stor
}

// Ignore any assignments of NULL.
GenTree* const data = store->Data()->gtSkipReloadOrCopy()->gtEffectiveVal();
GenTree* const data = store->Data()->gtSkipReloadOrCopy();
if (data->IsIntegralConst(0))
{
return WBF_NoBarrier;
Expand Down
34 changes: 34 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2482,6 +2482,34 @@ bool GenTreeCall::IsHelperCall(Compiler* compiler, unsigned helper) const
return IsHelperCall(compiler->eeFindHelper(helper));
}

//-------------------------------------------------------------------------
// IsRuntimeLookupHelperCall: Determine if this GT_CALL node represents a runtime lookup helper call.
//
// Arguments:
// compiler - the compiler instance so that we can call eeGetHelperNum
//
// Return Value:
// Returns true if this GT_CALL node represents a runtime lookup helper call.
//
bool GenTreeCall::IsRuntimeLookupHelperCall(Compiler* compiler) const
{
if (!IsHelperCall())
{
return false;
}

switch (compiler->eeGetHelperNum(gtCallMethHnd))
{
case CORINFO_HELP_RUNTIMEHANDLE_METHOD:
case CORINFO_HELP_RUNTIMEHANDLE_CLASS:
case CORINFO_HELP_RUNTIMEHANDLE_METHOD_LOG:
case CORINFO_HELP_RUNTIMEHANDLE_CLASS_LOG:
return true;
default:
return false;
}
}

//-------------------------------------------------------------------------
// IsSpecialIntrinsic: Determine if this GT_CALL node is a specific intrinsic.
//
Expand Down Expand Up @@ -2540,6 +2568,12 @@ bool GenTreeCall::Equals(GenTreeCall* c1, GenTreeCall* c2)
return false;
}

if (c1->IsHelperCall() && ((c1->gtCallMoreFlags & GTF_CALL_M_CAST_OBJ_NONNULL) !=
(c2->gtCallMoreFlags & GTF_CALL_M_CAST_OBJ_NONNULL)))
{
return false;
}

#ifdef FEATURE_READYTORUN
if (c1->gtEntryPoint.addr != c2->gtEntryPoint.addr)
{
Expand Down
21 changes: 5 additions & 16 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4101,11 +4101,12 @@ enum GenTreeCallFlags : unsigned int
GTF_CALL_M_GUARDED_DEVIRT_CHAIN = 0x00080000, // this call is a candidate for chained guarded devirtualization
GTF_CALL_M_ALLOC_SIDE_EFFECTS = 0x00100000, // this is a call to an allocator with side effects
GTF_CALL_M_SUPPRESS_GC_TRANSITION = 0x00200000, // suppress the GC transition (i.e. during a pinvoke) but a separate GC safe point is required.
GTF_CALL_M_EXP_RUNTIME_LOOKUP = 0x00400000, // [DEBUG only] this call needs to be transformed into CFG for the dynamic dictionary expansion feature.
GTF_CALL_M_EXPANDED_EARLY = 0x00800000, // the Virtual Call target address is expanded and placed in gtControlExpr in Morph rather than in Lower
GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x01000000, // this call has late devirtualzation info
GTF_CALL_M_LDVIRTFTN_INTERFACE = 0x02000000, // ldvirtftn on an interface type
GTF_CALL_M_CAST_CAN_BE_EXPANDED = 0x04000000, // this cast (helper call) can be expanded if it's profitable. To be removed.
GTF_CALL_M_CAST_OBJ_NONNULL = 0x08000000, // if we expand this specific cast we don't need to check the input object for null
// NOTE: if needed, this flag can be removed, and we can introduce new _NONNUL cast helpers
};

inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a)
Expand Down Expand Up @@ -4141,6 +4142,7 @@ enum GenTreeCallDebugFlags : unsigned int
GTF_CALL_MD_DEVIRTUALIZED = 0x00000002, // this call was devirtualized
GTF_CALL_MD_UNBOXED = 0x00000004, // this call was optimized to use the unboxed entry point
GTF_CALL_MD_GUARDED = 0x00000008, // this call was transformed by guarded devirtualization
GTF_CALL_MD_RUNTIME_LOOKUP_EXPANDED = 0x00000010, // this runtime lookup helper is expanded
};

inline constexpr GenTreeCallDebugFlags operator ~(GenTreeCallDebugFlags a)
Expand Down Expand Up @@ -5476,21 +5478,6 @@ struct GenTreeCall final : public GenTree
}
#endif

void SetExpRuntimeLookup()
{
gtCallMoreFlags |= GTF_CALL_M_EXP_RUNTIME_LOOKUP;
}

void ClearExpRuntimeLookup()
{
gtCallMoreFlags &= ~GTF_CALL_M_EXP_RUNTIME_LOOKUP;
}

bool IsExpRuntimeLookup() const
{
return (gtCallMoreFlags & GTF_CALL_M_EXP_RUNTIME_LOOKUP) != 0;
}

void SetExpandedEarly()
{
gtCallMoreFlags |= GTF_CALL_M_EXPANDED_EARLY;
Expand Down Expand Up @@ -5684,6 +5671,8 @@ struct GenTreeCall final : public GenTree

bool IsHelperCall(Compiler* compiler, unsigned helper) const;

bool IsRuntimeLookupHelperCall(Compiler* compiler) const;

bool IsSpecialIntrinsic(Compiler* compiler, NamedIntrinsic ni) const;

CorInfoHelpFunc GetHelperNum() const;
Expand Down
15 changes: 11 additions & 4 deletions src/coreclr/jit/helperexpansion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ GenTreeCall* Compiler::gtNewRuntimeLookupHelperCallNode(CORINFO_RUNTIME_LOOKUP*
// Leave a note that this method has runtime lookups we might want to expand (nullchecks, size checks) later.
// We can also consider marking current block as a runtime lookup holder to improve TP for Tier0
impInlineRoot()->setMethodHasExpRuntimeLookup();
helperCall->SetExpRuntimeLookup();
if (!impInlineRoot()->GetSignatureToLookupInfoMap()->Lookup(pRuntimeLookup->signature))
{
JITDUMP("Registering %p in SignatureToLookupInfoMap\n", pRuntimeLookup->signature)
Expand Down Expand Up @@ -152,13 +151,12 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm
{
BasicBlock* block = *pBlock;

if (!call->IsHelperCall() || !call->IsExpRuntimeLookup())
if (!call->IsRuntimeLookupHelperCall(this))
{
return false;
}

// Clear ExpRuntimeLookup flag so we won't miss any runtime lookup that needs partial expansion
call->ClearExpRuntimeLookup();
INDEBUG(call->gtCallDebugFlags |= GTF_CALL_MD_RUNTIME_LOOKUP_EXPANDED);

if (call->IsTailCall())
{
Expand Down Expand Up @@ -2405,6 +2403,15 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt,
assert(BasicBlock::sameEHRegion(firstBb, fallbackBb));
assert(BasicBlock::sameEHRegion(firstBb, lastTypeCheckBb));

// call guarantees that obj is never null, we can drop the nullcheck
// by converting it to a BBJ_ALWAYS to typeCheckBb.
if ((call->gtCallMoreFlags & GTF_CALL_M_CAST_OBJ_NONNULL) != 0)
{
fgRemoveStmt(nullcheckBb, nullcheckBb->lastStmt());
nullcheckBb->SetKindAndTarget(BBJ_ALWAYS, typeChecksBbs[0]);
fgRemoveRefPred(lastBb, nullcheckBb);
}

// Bonus step: merge prevBb with nullcheckBb as they are likely to be mergeable
if (fgCanCompactBlocks(firstBb, nullcheckBb))
{
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2203,7 +2203,8 @@ GenTree* Lowering::LowerCall(GenTree* node)
JITDUMP("\n");

// All runtime lookups are expected to be expanded in fgExpandRuntimeLookups
assert(!call->IsExpRuntimeLookup());
assert(!call->IsRuntimeLookupHelperCall(comp) ||
(call->gtCallDebugFlags & GTF_CALL_MD_RUNTIME_LOOKUP_EXPANDED) != 0);

// Also, always expand static cctor helper for NativeAOT, see
// /~https://github.com/dotnet/runtime/issues/68278#issuecomment-1543322819
Expand Down
Loading