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

JIT: Simple importer single-block forward substitution #109482

Closed
wants to merge 17 commits into from
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ Compiler::Compiler(ArenaAllocator* arena,
InlineInfo* inlineInfo)
: compArenaAllocator(arena)
, impInlineInfo(inlineInfo)
, impLclVals(LocalValMap(CompAllocator(arena, CMK_Generic)))
, impPendingBlockMembers(CompAllocator(arena, CMK_Generic))
, impSpillCliquePredMembers(CompAllocator(arena, CMK_Generic))
, impSpillCliqueSuccMembers(CompAllocator(arena, CMK_Generic))
Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -3070,7 +3070,7 @@ class Compiler
GenTree* gtNewGenericCon(var_types type, uint8_t* cnsVal);

GenTree* gtNewConWithPattern(var_types type, uint8_t pattern);

GenTreeLclVar* gtNewStoreLclVarNode(unsigned lclNum, GenTree* value);

GenTreeLclFld* gtNewStoreLclFldNode(
Expand Down Expand Up @@ -4719,6 +4719,14 @@ class Compiler
Statement* impStmtList = nullptr; // Statements for the BB being imported.
Statement* impLastStmt = nullptr; // The last statement for the current BB.

//------------------- Importer forward substitution -----------------------

typedef JitHashTable<UINT64, JitSmallPrimitiveKeyFuncs<UINT64>, GenTree*> LocalValMap;
LocalValMap impLclVals; // The values of the locals.

void impSetLclVal(unsigned lclNum, GenTree* tree);
bool impGetLclVal(GenTreeLclVar* tree, GenTree** val);

public:
static const unsigned CHECK_SPILL_ALL = static_cast<unsigned>(-1);
static const unsigned CHECK_SPILL_NONE = static_cast<unsigned>(-2);
Expand Down
97 changes: 97 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,100 @@ void Compiler::impAppendStmt(Statement* stmt)
impLastStmt = stmt;
}

//------------------------------------------------------------------------
// impSetLclVal: Add the local variable to the list of locals in the current block.
//
// Arguments:
// lclNum - The local variable number.
// tree - The tree that stores the value of the local variable.
//
void Compiler::impSetLclVal(unsigned lclNum, GenTree* tree)
{
assert(tree != nullptr);
if (!opts.OptimizationEnabled())
{
return;
}

impLclVals.Set((static_cast<UINT64>(compCurBB->bbNum) << 32 | lclNum), tree, LocalValMap::Overwrite);
JITDUMP("\nCreate substitution for V%02u at BB%02u in '%s':\n", lclNum, compCurBB->bbNum, info.compMethodName);
DISPTREE(tree);
}

//------------------------------------------------------------------------
// impGetLclVal: Get the value of the local variable from the list of locals in the current block.
//
// Arguments:
// tree - The local variable node.
// val - The value of the local variable.
//
// Return Value:
// True if the value of the local variable is found in the list of locals in the current block.
//
bool Compiler::impGetLclVal(GenTreeLclVar* tree, GenTree** val)
{
assert(tree != nullptr);

if (!opts.OptimizationEnabled())
{
return false;
}

GenTree* gtVal = nullptr;
INDEBUG(GenTreeLclVar* origTree = tree);

Compiler* comp = this;

auto setCompForLcl = [&comp](unsigned lclNum) {
if (comp->compIsForInlining())
{
for (unsigned i = 0; i < comp->impInlineInfo->argCnt; i++)
{
if (comp->impInlineInfo->inlArgInfo[i].argTmpNum == lclNum)
{
comp = comp->impInlineInfo->InlinerCompiler;
break;
}
}
}
};

setCompForLcl(tree->GetLclNum());

JITDUMP("\n");

while (true)
{
JITDUMP("Find substitution for V%02u at BB%02u in '%s'\n", tree->GetLclNum(), comp->compCurBB->bbNum,
comp->info.compMethodName);
LclVarDsc* lcl = comp->lvaGetDesc(tree->GetLclNum());
if (lcl->lvSingleDef && !lcl->lvHasLdAddrOp &&
comp->impLclVals.Lookup((static_cast<UINT64>(comp->compCurBB->bbNum) << 32 | tree->GetLclNum()), &gtVal) &&
gtVal->TypeIs(tree->TypeGet()))
{
if (gtVal->OperIs(GT_FTN_ADDR) || gtVal->OperIsConst() ||
(gtVal->IsCall() && (gtIsTypeHandleToRuntimeTypeHelper(gtVal->AsCall()) ||
gtIsTypeHandleToRuntimeTypeHandleHelper(gtVal->AsCall()))))
{
JITDUMP("Use substitution [%06u] for V%02u\n", gtVal->gtTreeID, tree->GetLclNum());
*val = gtVal;
return true;
}
else if (gtVal->OperIs(GT_LCL_VAR))
{
setCompForLcl(gtVal->AsLclVar()->GetLclNum());
tree = gtVal->AsLclVar();
continue;
}
}

break;
}

JITDUMP("No substitution found for V%02u\n", origTree->GetLclNum());
return false;
}

//------------------------------------------------------------------------
// impExtractLastStmt: Extract the last statement from the current stmts list.
//
Expand Down Expand Up @@ -667,6 +761,8 @@ void Compiler::impStoreToTemp(unsigned lclNum,

if (!store->IsNothingNode())
{
impSetLclVal(lclNum, val);

if (pAfterStmt)
{
Statement* storeStmt = gtNewStmt(store, di);
Expand Down Expand Up @@ -6847,6 +6943,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
impSpillSideEffects(false, CHECK_SPILL_ALL DEBUGARG("Spill before store to pinned local"));
}

impSetLclVal(lclNum, op1);
op1 = gtNewStoreLclVarNode(lclNum, op1);

// TODO-ASG: delete this zero-diff quirk. Requires some forward substitution work.
Expand Down
104 changes: 86 additions & 18 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,46 @@ var_types Compiler::impImportCall(OPCODE opcode,
// assume the worst-case.
mflags = (calliSig.callConv & CORINFO_CALLCONV_HASTHIS) ? 0 : CORINFO_FLG_STATIC;

if (opts.OptimizationEnabled() && call->AsCall()->gtCallAddr->OperIs(GT_FTN_ADDR))
{
pResolvedToken->hMethod = call->AsCall()->gtCallAddr->AsFptrVal()->gtFptrMethod;
pResolvedToken->hClass = info.compCompHnd->getMethodClass(pResolvedToken->hMethod);
eeGetCallInfo(pResolvedToken, nullptr, CORINFO_CALLINFO_LDFTN, callInfo);
// We only care about CALLCONV_HASTHIS for managed methods
unsigned targetFlags = (callInfo->sig.callConv & CORINFO_CALLCONV_HASTHIS) ? 0 : CORINFO_FLG_STATIC;
bool sigCompatible = callInfo->sig.numArgs == calliSig.numArgs && mflags == targetFlags &&
impCheckImplicitArgumentCoercion(JITtype2varType(callInfo->sig.retType),
JITtype2varType(calliSig.retType));

CORINFO_ARG_LIST_HANDLE methArg = callInfo->sig.args;
CORINFO_ARG_LIST_HANDLE callArg = calliSig.args;
for (unsigned i = 0; sigCompatible && i < callInfo->sig.numArgs; i++)
{
CORINFO_CLASS_HANDLE classHnd;
CorInfoType sigType = strip(info.compCompHnd->getArgType(&callInfo->sig, methArg, &classHnd));
CorInfoType argType = strip(info.compCompHnd->getArgType(&calliSig, callArg, &classHnd));
if (!impCheckImplicitArgumentCoercion(JITtype2varType(sigType), JITtype2varType(argType)))
{
sigCompatible = false;
break;
}

methArg = info.compCompHnd->getArgNext(methArg);
callArg = info.compCompHnd->getArgNext(callArg);
}

if (sigCompatible)
{
call = nullptr;
goto REGULAR_IMPORT;
}
else
{
// We have to clone the FTN_ADDR node, as it is substituted from the local
call->AsCall()->gtCallAddr = gtCloneExpr(call->AsCall()->gtCallAddr);
}
}

#ifdef DEBUG
if (verbose)
{
Expand All @@ -142,6 +182,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
}
else // (opcode != CEE_CALLI)
{
REGULAR_IMPORT:
NamedIntrinsic ni = NI_Illegal;

// Passing CORINFO_CALLINFO_ALLOWINSTPARAM indicates that this JIT is prepared to
Expand Down Expand Up @@ -1948,6 +1989,12 @@ GenTreeCall* Compiler::impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugI
// See ILCodeStream::LowerOpcode
assert(genActualType(fptr->gtType) == TYP_I_IMPL || genActualType(fptr->gtType) == TYP_INT);

GenTree* fptrVal;
if (fptr->OperIs(GT_LCL_VAR) && impGetLclVal(fptr->AsLclVar(), &fptrVal) && fptrVal->OperIs(GT_FTN_ADDR))
{
fptr = fptrVal;
}

#ifdef DEBUG
// This temporary must never be converted to a double in stress mode,
// because that can introduce a call to the cast helper after the
Expand Down Expand Up @@ -3414,6 +3461,15 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
CorInfoType callJitType = sig->retType;
var_types callType = JITtype2varType(callJitType);

auto getSubstitutedOp = [&](GenTree* op) -> GenTree* {
GenTree* opVal = nullptr;
if (op->OperIs(GT_LCL_VAR) && impGetLclVal(op->AsLclVar(), &opVal))
{
return opVal;
}
return op;
};

/* First do the intrinsics which are always smaller than a call */

if (ni != NI_Illegal)
Expand Down Expand Up @@ -3780,7 +3836,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,

case NI_System_RuntimeTypeHandle_ToIntPtr:
{
GenTree* op1 = impStackTop(0).val;
GenTree* op1 = getSubstitutedOp(impStackTop(0).val);

if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHandleHelper(op1->AsCall()))
{
Expand Down Expand Up @@ -3808,7 +3864,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_RuntimeType_get_TypeHandle)
{
// Check that the arg is CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE helper call
GenTree* arg = call->gtArgs.GetArgByIndex(0)->GetNode();
GenTree* arg = getSubstitutedOp(call->gtArgs.GetArgByIndex(0)->GetNode());
if (arg->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(arg->AsCall()))
{
impPopStack();
Expand All @@ -3818,7 +3874,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
op1->AsRetExpr()->gtInlineCandidate->gtBashToNOP();
}
// Skip roundtrip and return the type handle directly
retNode = arg->AsCall()->gtArgs.GetArgByIndex(0)->GetNode();
retNode = gtCloneExpr(arg->AsCall()->gtArgs.GetArgByIndex(0)->GetNode());
}
}
}
Expand All @@ -3827,7 +3883,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,

case NI_System_Type_GetTypeFromHandle:
{
GenTree* op1 = impStackTop(0).val;
GenTree* op1 = getSubstitutedOp(impStackTop(0).val);
CorInfoHelpFunc typeHandleHelper;
if (op1->gtOper == GT_CALL && op1->AsCall()->IsHelperCall() &&
gtIsTypeHandleToRuntimeTypeHandleHelper(op1->AsCall(), &typeHandleHelper))
Expand Down Expand Up @@ -3859,7 +3915,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,

case NI_System_Type_GetGenericTypeDefinition:
{
GenTree* type = impStackTop(0).val;
GenTree* type = getSubstitutedOp(impStackTop(0).val);

retNode = impGetGenericTypeDefinition(type);
break;
Expand Down Expand Up @@ -3922,17 +3978,17 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,

case NI_System_Type_IsAssignableFrom:
{
GenTree* typeTo = impStackTop(1).val;
GenTree* typeFrom = impStackTop(0).val;
GenTree* typeTo = getSubstitutedOp(impStackTop(1).val);
GenTree* typeFrom = getSubstitutedOp(impStackTop(0).val);

retNode = impTypeIsAssignable(typeTo, typeFrom);
break;
}

case NI_System_Type_IsAssignableTo:
{
GenTree* typeTo = impStackTop(0).val;
GenTree* typeFrom = impStackTop(1).val;
GenTree* typeTo = getSubstitutedOp(impStackTop(0).val);
GenTree* typeFrom = getSubstitutedOp(impStackTop(1).val);

retNode = impTypeIsAssignable(typeTo, typeFrom);
break;
Expand All @@ -3944,7 +4000,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
//
// struct RuntimeTypeHandle { IntPtr _value; }
//
GenTree* op1 = impStackTop(0).val;
GenTree* op1 = getSubstitutedOp(impStackTop(0).val);
if (IsTargetAbi(CORINFO_NATIVEAOT_ABI) && op1->IsHelperCall() &&
gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && callvirt)
{
Expand Down Expand Up @@ -3977,7 +4033,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
// e.g., `typeof(int).IsValueType` => `true`
// e.g., `typeof(Span<int>).IsByRefLike` => `true`
CORINFO_CLASS_HANDLE hClass = NO_CLASS_HANDLE;
if (gtIsTypeof(impStackTop().val, &hClass))
if (gtIsTypeof(getSubstitutedOp(impStackTop().val), &hClass))
{
assert(hClass != NO_CLASS_HANDLE);
switch (ni)
Expand Down Expand Up @@ -4036,7 +4092,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,

case NI_System_Type_GetEnumUnderlyingType:
{
GenTree* type = impStackTop().val;
GenTree* type = getSubstitutedOp(impStackTop().val);
CORINFO_CLASS_HANDLE hClassEnum = NO_CLASS_HANDLE;
CORINFO_CLASS_HANDLE hClassUnderlying = NO_CLASS_HANDLE;
if (gtIsTypeof(type, &hClassEnum) && (hClassEnum != NO_CLASS_HANDLE) &&
Expand Down Expand Up @@ -9597,20 +9653,32 @@ GenTree* Compiler::impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
assert(varTypeIsFloating(callType));
assert(sig->numArgs == 2);

GenTreeDblCon* cnsNode = nullptr;
GenTree* otherNode = nullptr;
GenTreeDblCon* cnsNode = nullptr;
GenTree* otherNode = nullptr;
GenTree* substituteNode = nullptr;

GenTree* op2 = impStackTop().val;
if (op2->OperIs(GT_LCL_VAR) && impGetLclVal(op2->AsLclVar(), &substituteNode))
{
op2 = substituteNode;
}
op2 = impImplicitR4orR8Cast(op2, callType);

GenTree* op2 = impImplicitR4orR8Cast(impStackTop().val, callType);
GenTree* op1 = impImplicitR4orR8Cast(impStackTop(1).val, callType);
GenTree* op1 = impStackTop(1).val;
if (op1->OperIs(GT_LCL_VAR) && impGetLclVal(op1->AsLclVar(), &substituteNode))
{
op1 = substituteNode;
}
op1 = impImplicitR4orR8Cast(op1, callType);

if (op2->IsCnsFltOrDbl())
{
cnsNode = op2->AsDblCon();
cnsNode = gtCloneExpr(op2)->AsDblCon();
otherNode = op1;
}
else if (op1->IsCnsFltOrDbl())
{
cnsNode = op1->AsDblCon();
cnsNode = gtCloneExpr(op1)->AsDblCon();
otherNode = op2;
}

Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7873,6 +7873,13 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller,
goto exit;
}

if (pCallee->HasUnmanagedCallersOnlyAttribute() && NDirect::MarshalingRequired(pCallee))
{
result = INLINE_NEVER;
szFailReason = "Inlinee is marked as unmanaged callers only but requires runtime marshaling";
goto exit;
}

// Also check to see if the method requires a security object. This means they call demand and
// shouldn't be inlined.
if (IsMdRequireSecObject(pCallee->GetAttrs()))
Expand Down
Loading