Skip to content

Inline properties during importation #96325

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Inline properties during importation #96325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9afe6f6
Inline properties during importation
Suchiman Dec 25, 2023
1c7b26e
Merge branch 'main' into alwaysInlineProps
EgorBo Sep 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4541,6 +4541,8 @@ class Compiler
void impHandleAccessAllowed(CorInfoIsAccessAllowedResult result, CORINFO_HELPER_DESC* helperCall);
void impHandleAccessAllowedInternal(CorInfoIsAccessAllowedResult result, CORINFO_HELPER_DESC* helperCall);

bool impTryFindField(CORINFO_METHOD_HANDLE methHnd, CORINFO_RESOLVED_TOKEN* pResolvedToken, OPCODE* opcode);

var_types impImportCall(OPCODE opcode,
CORINFO_RESOLVED_TOKEN* pResolvedToken,
CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, // Is this a "constrained." call on a
Expand Down Expand Up @@ -11126,8 +11128,8 @@ class Compiler

void compProcessScopesUntil(unsigned offset,
VARSET_TP* inScope,
void (Compiler::*enterScopeFn)(VARSET_TP* inScope, VarScopeDsc*),
void (Compiler::*exitScopeFn)(VARSET_TP* inScope, VarScopeDsc*));
void (Compiler::*enterScopeFn)(VARSET_TP* inScope, VarScopeDsc*),
void (Compiler::*exitScopeFn)(VARSET_TP* inScope, VarScopeDsc*));

#ifdef DEBUG
void compDispScopeLists();
Expand Down
58 changes: 45 additions & 13 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6490,6 +6490,8 @@ void Compiler::impImportBlockCode(BasicBlock* block)
return;
}

CORINFO_METHOD_HANDLE callerProp = nullptr;

/* Get the size of additional parameters */

signed int sz = opcodeSizes[opcode];
Expand Down Expand Up @@ -8985,6 +8987,26 @@ void Compiler::impImportBlockCode(BasicBlock* block)
combine(combine(CORINFO_CALLINFO_ALLOWINSTPARAM, CORINFO_CALLINFO_SECURITYCHECKS),
(opcode == CEE_CALLVIRT) ? CORINFO_CALLINFO_CALLVIRT : CORINFO_CALLINFO_NONE),
&callInfo);

if (!opts.compDbgCode && callInfo.kind == CORINFO_CALL &&
!(callInfo.methodFlags & (CORINFO_FLG_DONT_INLINE | CORINFO_FLG_DONT_INLINE_CALLER)) &&
!(prefixFlags & PREFIX_CONSTRAINED) &&
((callInfo.sig.retType == CORINFO_TYPE_VOID && callInfo.sig.numArgs == 1) || // possible setter
(callInfo.sig.retType > CORINFO_TYPE_VOID && callInfo.sig.numArgs == 0))) // possible getter
{
if (impTryFindField(callInfo.hMethod, &resolvedToken, &opcode))
{
callerProp = callInfo.hMethod;
if (opcode == CEE_LDFLD || opcode == CEE_LDSFLD)
{
goto LOADFIELD;
}
else
{
goto STOREFIELD;
}
}
}
}
else
{
Expand Down Expand Up @@ -9151,19 +9173,19 @@ void Compiler::impImportBlockCode(BasicBlock* block)
case CEE_LDFLDA:
case CEE_LDSFLDA:
{
bool isLoadAddress = (opcode == CEE_LDFLDA || opcode == CEE_LDSFLDA);
bool isLoadStatic = (opcode == CEE_LDSFLD || opcode == CEE_LDSFLDA);

/* Get the CP_Fieldref index */
assertImp(sz == sizeof(unsigned));

_impResolveToken(CORINFO_TOKENKIND_Field);

LOADFIELD:
JITDUMP(" %08X", resolvedToken.token);

GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags);
int aflags = isLoadAddress ? CORINFO_ACCESS_ADDRESS : CORINFO_ACCESS_GET;
GenTree* obj = nullptr;
bool isLoadAddress = (opcode == CEE_LDFLDA || opcode == CEE_LDSFLDA);
bool isLoadStatic = (opcode == CEE_LDSFLD || opcode == CEE_LDSFLDA);
GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags);
int aflags = isLoadAddress ? CORINFO_ACCESS_ADDRESS : CORINFO_ACCESS_GET;
GenTree* obj = nullptr;

if ((opcode == CEE_LDFLD) || (opcode == CEE_LDFLDA))
{
Expand All @@ -9175,7 +9197,12 @@ void Compiler::impImportBlockCode(BasicBlock* block)
}
}

eeGetFieldInfo(&resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo);
auto callerHandle = callerProp;
if (!callerHandle)
{
callerHandle = info.compMethodHnd;
}
info.compCompHnd->getFieldInfo(&resolvedToken, callerHandle, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo);

// Note we avoid resolving the normalized (struct) type just yet; we may not need it (for ld[s]flda).
lclTyp = JITtype2varType(fieldInfo.fieldType);
Expand Down Expand Up @@ -9421,21 +9448,26 @@ void Compiler::impImportBlockCode(BasicBlock* block)
case CEE_STFLD:
case CEE_STSFLD:
{
bool isStoreStatic = (opcode == CEE_STSFLD);

/* Get the CP_Fieldref index */

assertImp(sz == sizeof(unsigned));

_impResolveToken(CORINFO_TOKENKIND_Field);

STOREFIELD:
JITDUMP(" %08X", resolvedToken.token);

GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags);
int aflags = CORINFO_ACCESS_SET;
GenTree* obj = nullptr;
bool isStoreStatic = (opcode == CEE_STSFLD);
GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags);
int aflags = CORINFO_ACCESS_SET;
GenTree* obj = nullptr;

eeGetFieldInfo(&resolvedToken, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo);
auto callerHandle = callerProp;
if (!callerHandle)
{
callerHandle = info.compMethodHnd;
}
info.compCompHnd->getFieldInfo(&resolvedToken, callerHandle, (CORINFO_ACCESS_FLAGS)aflags, &fieldInfo);

ClassLayout* layout;
lclTyp = TypeHandleToVarType(fieldInfo.fieldType, fieldInfo.structType, &layout);
Expand Down
128 changes: 128 additions & 0 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8833,6 +8833,134 @@ bool Compiler::impTailCallRetTypeCompatible(bool allowWideni
return false;
}

//------------------------------------------------------------------------
// impTryFindField: try to find and resolve the backing field of an auto property
//
// Arguments:
// methHnd - inline candidate
// pResolvedToken - resolved field info will be stored here
// opcode - opcode that was used against the field will be stored here
//
// Notes:
// Will update inlineResult with observations and possible failure
// status (if method cannot be inlined)
//
bool Compiler::impTryFindField(CORINFO_METHOD_HANDLE methHnd, CORINFO_RESOLVED_TOKEN* pResolvedToken, OPCODE* opcode)
{
// Either EE or JIT might throw exceptions below.
// If that happens, just don't inline the method.
//
struct Param
{
Compiler* pThis;
CORINFO_METHOD_HANDLE fncHandle;
CORINFO_CONTEXT_HANDLE exactContextHnd;
CORINFO_RESOLVED_TOKEN resolvedToken;
OPCODE opcode;
bool success;
} param;
memset(&param, 0, sizeof(param));

param.pThis = this;
param.fncHandle = methHnd;
param.exactContextHnd = MAKE_METHODCONTEXT(methHnd);
param.opcode = CEE_ILLEGAL;
param.success = false;

info.compCompHnd->beginInlining(info.compMethodHnd, methHnd);

bool success = eeRunWithErrorTrap<Param>(
[](Param* pParam) {
// Cache some frequently accessed state.
//
Compiler* const compiler = pParam->pThis;
COMP_HANDLE compCompHnd = compiler->info.compCompHnd;
CORINFO_METHOD_HANDLE ftn = pParam->fncHandle;

#ifdef DEBUG
if (JitConfig.JitNoInline())
{
compCompHnd->reportInliningDecision(compiler->info.compMethodHnd, ftn, INLINE_FAIL, "JitNoInline is set");
return;
}
#endif

// Fetch method info. This may fail, if the method doesn't have IL.
//
CORINFO_METHOD_INFO methInfo;
if (!compCompHnd->getMethodInfo(pParam->fncHandle, &methInfo, pParam->exactContextHnd))
{
compCompHnd->reportInliningDecision(compiler->info.compMethodHnd, ftn, INLINE_FAIL, "getMethodInfo failed");
return;
}

pParam->resolvedToken.tokenContext = pParam->exactContextHnd;
pParam->resolvedToken.tokenScope = methInfo.scope;
pParam->resolvedToken.tokenType = CORINFO_TOKENKIND_Field;

auto code = methInfo.ILCode;
auto size = methInfo.ILCodeSize;
// instance getter
if (size == 7 && code[0] == CEE_LDARG_0 && code[1] == CEE_LDFLD && code[6] == CEE_RET)
{
pParam->resolvedToken.token = getU4LittleEndian(&code[2]);
pParam->opcode = CEE_LDFLD;
}
// instance setter
else if (size == 8 && code[0] == CEE_LDARG_0 && code[1] == CEE_LDARG_1 && code[2] == CEE_STFLD &&
code[7] == CEE_RET)
{
pParam->resolvedToken.token = getU4LittleEndian(&code[3]);
pParam->opcode = CEE_STFLD;
}
else
{
compCompHnd->reportInliningDecision(compiler->info.compMethodHnd, ftn, INLINE_FAIL, "Not a property");
return;
}

// Speculatively check if initClass() can be done.
// If it can be done, we will try to inline the method.
CorInfoInitClassResult const initClassResult =
compCompHnd->initClass(nullptr /* field */, ftn /* method */, pParam->exactContextHnd /* context */);

if (initClassResult & CORINFO_INITCLASS_DONT_INLINE)
{
compCompHnd->reportInliningDecision(compiler->info.compMethodHnd, ftn, INLINE_FAIL,
"InitClass reported don't inline");
return;
}

// Given the VM the final say in whether to inline or not.
// This should be last since for verifiable code, this can be expensive
// This will call reportInliningDecision for us if inlining is not allowed
CorInfoInline const vmResult = compCompHnd->canInline(compiler->info.compMethodHnd, ftn);
if (vmResult == INLINE_FAIL || vmResult == INLINE_NEVER)
{
return;
}

compCompHnd->resolveToken(&pParam->resolvedToken);
pParam->success = true;
},
&param);

if (success && param.success)
{
*opcode = param.opcode;
*pResolvedToken = param.resolvedToken;
info.compCompHnd->reportInliningDecision(info.compMethodHnd, methHnd, INLINE_PASS,
"Trivial property backing field has been inlined");
return true;
}
if (!success)
{
info.compCompHnd->reportInliningDecision(info.compMethodHnd, methHnd, INLINE_FAIL,
"Failed to inline trivial property");
}
return false;
}

//------------------------------------------------------------------------
// impCheckCanInline: do more detailed checks to determine if a method can
// be inlined, and collect information that will be needed later
Expand Down
Loading