Skip to content

JIT: Improve inliner (constant feeds constant test, foldable calls) #50784

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 4 commits into from
Closed

JIT: Improve inliner (constant feeds constant test, foldable calls) #50784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b06d502
Improve classic inliner with foldable calls
EgorBo Apr 6, 2021
f25b371
Update fgbasic.cpp
EgorBo Apr 6, 2021
753e517
Update fgbasic.cpp
EgorBo Apr 6, 2021
82d07a0
Update fgbasic.cpp
EgorBo Apr 6, 2021
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
27 changes: 26 additions & 1 deletion src/coreclr/jit/fgbasic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,18 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed

if ((methodFlags & CORINFO_FLG_JIT_INTRINSIC) != 0)
{
if ((intrinsicID == CORINFO_INTRINSIC_StringLength) && pushedStack.IsStackAtLeastOneDeep())
{
unsigned slot0 = pushedStack.GetSlot0();

if (FgStack::IsConstant(slot0) ||
(FgStack::IsArgument(slot0) && isInlining &&
impInlineInfo->inlArgInfo[FgStack::SlotTypeToArgNum(slot0)].argIsInvariant))
{
// slot0 is an ldstr or an arg which is a string literal
compInlineResult->Note(InlineObservation::CALLEE_FOLDABLE_CALL);
}
}
if (intrinsicID == CORINFO_INTRINSIC_Illegal)
{
ni = lookupNamedIntrinsic(methodHnd);
Expand All @@ -997,6 +1009,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
case NI_IsSupported_True:
case NI_IsSupported_False:
{
compInlineResult->Note(InlineObservation::CALLEE_FOLDABLE_CALL);
pushedStack.PushConstant();
break;
}
Expand Down Expand Up @@ -1733,7 +1746,12 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
}
}
}

// e.g. "if (Avx.IsSupported)" where get_IsSupported() was recognized as a run-time const
else if (FgStack::IsConstant(slot0))
{
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST);
compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST);
}
return;
}
}
Expand All @@ -1754,6 +1772,13 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST);
}

// Constant feeds constant test
if (FgStack::IsConstant(slot0) && FgStack::IsConstant(slot1))
{
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST);
compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST);
}

// Arg feeds range check
if ((FgStack::IsArrayLen(slot0) && FgStack::IsArgument(slot1)) ||
(FgStack::IsArrayLen(slot1) && FgStack::IsArgument(slot0)))
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/inline.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ INLINE_OBSERVATION(TOO_MUCH_IL, bool, "too many il bytes",

// ------ Callee Information -------

INLINE_OBSERVATION(FOLDABLE_CALL, bool, "foldable call", INFORMATION, CALLEE)
Copy link
Member

@tannergooding tannergooding Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some future PR, we could also track the intrinsic id here.

This would allow us to determine, at least the following interesting pieces:

  • Number of intrinsic calls
  • Number of hardware intrinsic calls
  • Number of Type.op_Equality calls

There may be other interesting bits as well, but these are at least ones that have one or more of:

  • significantly decreased cost (hwintrinsics are instructions not calls)
  • additional optimizations (many intrinsics have CSE support)
  • potential dead code elimination (type equality checks, particularly where T : struct)

INLINE_OBSERVATION(ARG_FEEDS_CONSTANT_TEST, bool, "argument feeds constant test", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_TEST, bool, "argument feeds test", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_RANGE_CHECK, bool, "argument feeds range check", INFORMATION, CALLEE)
Expand Down
17 changes: 14 additions & 3 deletions src/coreclr/jit/inlinepolicy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,13 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
break;

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

case InlineObservation::CALLEE_FOLDABLE_CALL:
m_FoldableCall++;
break;

case InlineObservation::CALLEE_BEGIN_OPCODE_SCAN:
{
// Set up the state machine, if this inline is
Expand Down Expand Up @@ -720,6 +721,15 @@ double DefaultPolicy::DetermineMultiplier()
JITDUMP("\nInline candidate has arg that feeds range check. Multiplier increased to %g.", multiplier);
}

if (m_FoldableCall > 0)
{
// TBD:
// Do we want to increase the multiplier here?
// At least we should compensate increased calleeNativeSizeEstimate because of "call" instructions
// (weight = 79) which actually are guaranteed to be folded.
JITDUMP("\nInline candidate has %d call(s) which is guaranteed to be folded", m_FoldableCall);
}

if (m_ConstantArgFeedsConstantTest > 0)
{
multiplier += 3.0;
Expand Down Expand Up @@ -2017,6 +2027,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_FoldableCall);
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/jit/inlinepolicy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class DefaultPolicy : public LegalPolicy
, m_ArgFeedsConstantTest(0)
, m_ArgFeedsRangeCheck(0)
, m_ConstantArgFeedsConstantTest(0)
, m_FoldableCall(0)
, m_CalleeNativeSizeEstimate(0)
, m_CallsiteNativeSizeEstimate(0)
, m_IsForceInline(false)
Expand Down Expand Up @@ -164,6 +165,7 @@ class DefaultPolicy : public LegalPolicy
unsigned m_ArgFeedsConstantTest;
unsigned m_ArgFeedsRangeCheck;
unsigned m_ConstantArgFeedsConstantTest;
unsigned m_FoldableCall;
int m_CalleeNativeSizeEstimate;
int m_CallsiteNativeSizeEstimate;
bool m_IsForceInline : 1;
Expand Down