Skip to content

Commit

Permalink
Add "Inline candidate has const arg that feeds string.Length." heuristic
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo committed Mar 7, 2020
1 parent 5c981af commit 3810c21
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 1 deletion.
45 changes: 44 additions & 1 deletion src/coreclr/src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4171,6 +4171,10 @@ class FgStack
{
Push(SLOT_ARRAYLEN);
}
void PushStringLen()
{
Push(SLOT_STRINGLEN);
}
void PushArgument(unsigned arg)
{
Push(SLOT_ARGUMENT + arg);
Expand All @@ -4193,6 +4197,10 @@ class FgStack
{
return value == SLOT_ARRAYLEN;
}
static bool IsStringLen(unsigned value)
{
return value == SLOT_STRINGLEN;
}
static bool IsArgument(unsigned value)
{
return value >= SLOT_ARGUMENT;
Expand Down Expand Up @@ -4222,7 +4230,8 @@ class FgStack
SLOT_UNKNOWN = 0,
SLOT_CONSTANT = 1,
SLOT_ARRAYLEN = 2,
SLOT_ARGUMENT = 3
SLOT_STRINGLEN = 3,
SLOT_ARGUMENT = 4,
};

void Push(int type)
Expand Down Expand Up @@ -4479,6 +4488,31 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
compInlineResult->Note(InlineObservation::CALLEE_LOOKS_LIKE_WRAPPER);
}
}

if (isInlining && makeInlineObservations && opcode == CEE_CALLVIRT)
{
CORINFO_RESOLVED_TOKEN resolvedToken;
impResolveToken(codeAddr, &resolvedToken, CORINFO_TOKENKIND_Method);

CORINFO_CALL_INFO corinfo;
info.compCompHnd->getCallInfo(&resolvedToken, nullptr, info.compMethodHnd, CORINFO_CALLINFO_CALLVIRT, &corinfo);

// ugly hack to check if this CEE_CALLVIRT is actually `string.get_Length`
if (corinfo.hMethod != nullptr &&
corinfo.hMethod != (CORINFO_METHOD_HANDLE)0xcccccccccccccccc) //isInvalidHandle
{
const char* className;
const char* methodName = eeGetMethodName(corinfo.hMethod, &className);
if (!strcmp("System.String", className) && !strcmp("get_Length", methodName))
{
if (makeInlineObservations)
{
pushedStack.PushStringLen();
}
}
}
}

}
break;

Expand Down Expand Up @@ -5150,6 +5184,15 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_RANGE_CHECK);
}

// Const string feeds String.Length
if ((FgStack::IsConstant(slot0) && FgStack::IsStringLen(slot1)) ||
(FgStack::IsConstant(slot1) && FgStack::IsStringLen(slot0)))
{
compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_STRLEN);
// TODO: check `oper` and `slot3` for const to apply the observation only const tests, e.g.:
// `cnsStr.Length > 0` (CEE_CEQ, CEE_CGT, etc..)
}

// Check for an incoming arg that's a constant
if (isInlining)
{
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/inline.def
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ INLINE_OBSERVATION(RARE_GC_STRUCT, bool, "rarely called, has gc str
// ------ Call Site Information -------

INLINE_OBSERVATION(CONSTANT_ARG_FEEDS_TEST, bool, "constant argument feeds test", INFORMATION, CALLSITE)
INLINE_OBSERVATION(CONSTANT_ARG_FEEDS_STRLEN, bool, "constant argument feeds StrLen",INFORMATION, CALLSITE)
INLINE_OBSERVATION(DEPTH, int, "depth", INFORMATION, CALLSITE)
INLINE_OBSERVATION(FREQUENCY, int, "rough call site frequency", INFORMATION, CALLSITE)
INLINE_OBSERVATION(IN_LOOP, bool, "call site is in a loop", INFORMATION, CALLSITE)
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/src/jit/inlinepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,13 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
m_ConstantArgFeedsConstantTest++;
break;

case InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_STRLEN:
// We shouldn't see this for a prejit root since
// we don't know anything about callers.
assert(!m_IsPrejitRoot);
m_ConstantArgFeedsStrLen++;
break;

case InlineObservation::CALLEE_BEGIN_OPCODE_SCAN:
{
// Set up the state machine, if this inline is
Expand Down Expand Up @@ -658,6 +665,13 @@ double DefaultPolicy::DetermineMultiplier()
multiplier += 3.0;
JITDUMP("\nInline candidate has const arg that feeds a conditional. Multiplier increased to %g.", multiplier);
}

if (m_ConstantArgFeedsStrLen > 0)
{
multiplier += 7.0;
JITDUMP("\nInline candidate has const arg that feeds string.Length. Multiplier increased to %g.", multiplier);
}

// For prejit roots we do not see the call sites. To be suitably optimistic
// assume that call sites may pass constants.
else if (m_IsPrejitRoot && ((m_ArgFeedsConstantTest > 0) || (m_ArgFeedsTest > 0)))
Expand Down Expand Up @@ -1729,6 +1743,7 @@ void DiscretionaryPolicy::EstimateCodeSize()
-0.238 * m_IsInstanceCtor +
-5.357 * m_IsFromPromotableValueClass +
-7.901 * (m_ConstantArgFeedsConstantTest > 0 ? 1 : 0) +
-7.901 * (m_ConstantArgFeedsStrLen > 0 ? 1 : 0) +
0.065 * m_CalleeNativeSizeEstimate;
// clang-format on

Expand Down Expand Up @@ -1930,6 +1945,7 @@ void DiscretionaryPolicy::DumpData(FILE* file) const
fprintf(file, ",%u", m_MethodIsMostlyLoadStore ? 1 : 0);
fprintf(file, ",%u", m_ArgFeedsRangeCheck);
fprintf(file, ",%u", m_ConstantArgFeedsConstantTest);
fprintf(file, ",%u", m_ConstantArgFeedsStrLen);
fprintf(file, ",%d", m_CalleeNativeSizeEstimate);
fprintf(file, ",%d", m_CallsiteNativeSizeEstimate);
fprintf(file, ",%d", m_ModelCodeSizeEstimate);
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/inlinepolicy.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class DefaultPolicy : public LegalPolicy
, m_ArgFeedsConstantTest(0)
, m_ArgFeedsRangeCheck(0)
, m_ConstantArgFeedsConstantTest(0)
, m_ConstantArgFeedsStrLen(0)
, m_CalleeNativeSizeEstimate(0)
, m_CallsiteNativeSizeEstimate(0)
, m_IsForceInline(false)
Expand Down Expand Up @@ -160,6 +161,7 @@ class DefaultPolicy : public LegalPolicy
unsigned m_ArgFeedsConstantTest;
unsigned m_ArgFeedsRangeCheck;
unsigned m_ConstantArgFeedsConstantTest;
unsigned m_ConstantArgFeedsStrLen;
int m_CalleeNativeSizeEstimate;
int m_CallsiteNativeSizeEstimate;
bool m_IsForceInline : 1;
Expand Down

0 comments on commit 3810c21

Please sign in to comment.