Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
Improve Inliner, vol.3
  • Loading branch information
EgorBo committed Jul 10, 2021
commit d711b0f1970574a83857e1f00fb5d67008c40403
14 changes: 12 additions & 2 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1347,13 +1347,19 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
handled = true;
compInlineResult->Note(InlineObservation::CALLSITE_FOLDABLE_EXPR);
}
if ((FgStack::IsConstant(arg0) && FgStack::IsConstant(arg1)) ||
(FgStack::IsConstant(arg1) && FgStack::IsConstant(arg0)))
else if ((FgStack::IsConstant(arg0) && FgStack::IsConstant(arg1)) ||
(FgStack::IsConstant(arg1) && FgStack::IsConstant(arg0)))
{
// both are constants, but we're mostly interested in cases where a const arg leads to
// a foldable expression.
handled = true;
}
else if ((FgStack::IsArgument(arg0) == FgStack::IsArgument(arg1)) && (arg0 == arg1))
{
// Both args are the same
handled = true;
compInlineResult->Note(InlineObservation::CALLSITE_FOLDABLE_EXPR);
}
else if (FgStack::IsArgument(arg0) && FgStack::IsConstantOrConstArg(arg1, impInlineInfo))
{
// "Arg op CNS" --> keep arg0 in the stack for the next ops
Expand Down Expand Up @@ -1583,6 +1589,10 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
if (makeInlineObservations)
{
compInlineResult->Note(InlineObservation::CALLEE_HAS_SWITCH);
if (FgStack::IsConstantOrConstArg(pushedStack.Top(), impInlineInfo))
{
compInlineResult->Note(InlineObservation::CALLSITE_FOLDABLE_SWITCH);
}

// Fail fast, if we're inlining and can't handle this.
if (isInlining && compInlineResult->IsFailure())
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13258,8 +13258,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
goto COND_JUMP;

case CEE_SWITCH:
assert(!compIsForInlining());

if (tiVerificationNeeded)
{
Verify(impStackTop().seTypeInfo.IsType(TI_INT), "Bad switch val");
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/inline.def
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ INLINE_OBSERVATION(FOLDABLE_INTRINSIC, int, "foldable intrinsic",
INLINE_OBSERVATION(FOLDABLE_EXPR, int, "foldable binary expression", INFORMATION, CALLSITE)
INLINE_OBSERVATION(FOLDABLE_EXPR_UN, int, "foldable unary expression", INFORMATION, CALLSITE)
INLINE_OBSERVATION(FOLDABLE_BRANCH, int, "foldable branch", INFORMATION, CALLSITE)
INLINE_OBSERVATION(FOLDABLE_SWITCH, int, "foldable switch", INFORMATION, CALLSITE)
INLINE_OBSERVATION(DIV_BY_CNS, int, "dividy by const", INFORMATION, CALLSITE)
INLINE_OBSERVATION(CONSTANT_ARG_FEEDS_TEST, bool, "constant argument feeds test", INFORMATION, CALLSITE)
INLINE_OBSERVATION(DEPTH, int, "depth", INFORMATION, CALLSITE)
Expand Down
40 changes: 35 additions & 5 deletions src/coreclr/jit/inlinepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
m_ArgFeedsRangeCheck++;
break;

case InlineObservation::CALLEE_HAS_SWITCH:
case InlineObservation::CALLEE_UNSUPPORTED_OPCODE:
propagate = true;
break;
Expand Down Expand Up @@ -1294,6 +1293,14 @@ void ExtendedDefaultPolicy::NoteBool(InlineObservation obs, bool value)
m_FoldableBranch++;
break;

case InlineObservation::CALLSITE_FOLDABLE_SWITCH:
m_FoldableSwitch++;
break;

case InlineObservation::CALLEE_HAS_SWITCH:
m_Switch++;
break;

case InlineObservation::CALLSITE_DIV_BY_CNS:
m_DivByCns++;
break;
Expand Down Expand Up @@ -1364,9 +1371,9 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
{
// We're not able to recognize arg-specific foldable branches
// in prejit-root mode.
bbLimit += 3;
bbLimit += 5 + m_Switch * 10;
}
bbLimit += m_FoldableBranch;
bbLimit += m_FoldableBranch * 2 + m_ThrowBlock * 2 + m_FoldableSwitch * 10;
if ((unsigned)value > bbLimit)
{
SetNever(InlineObservation::CALLEE_TOO_MANY_BASIC_BLOCKS);
Expand Down Expand Up @@ -1457,7 +1464,7 @@ double ExtendedDefaultPolicy::DetermineMultiplier()

if (m_NonGenericCallsGeneric)
{
multiplier += 1.5;
multiplier += 2.0;
JITDUMP("\nInline candidate is generic and caller is not. Multiplier increased to %g.", multiplier);
}

Expand Down Expand Up @@ -1507,7 +1514,7 @@ double ExtendedDefaultPolicy::DetermineMultiplier()
if (m_Intrinsic > 0)
{
// In most cases such intrinsics are lowered as single CPU instructions
multiplier += 1.5;
multiplier += 1.0 + m_Intrinsic * 0.2;
JITDUMP("\nInline has %d intrinsics. Multiplier increased to %g.", m_Intrinsic, multiplier);
}

Expand Down Expand Up @@ -1636,6 +1643,27 @@ double ExtendedDefaultPolicy::DetermineMultiplier()
break;
}

if (m_FoldableSwitch > 0)
{
multiplier += m_FoldableSwitch * 5.0;
JITDUMP("\nInline candidate has %d foldable switches. Multiplier increased to %g.", m_FoldableSwitch, multiplier);
}
else if (m_Switch > 0)
{
if (m_IsPrejitRoot)
{
// Assume the switches can be foldable in PrejitRoot mode.
multiplier += m_Switch * 5.0;
JITDUMP("\nPrejit root candidate has %d switches. Multiplier increased to %g.", m_Switch, multiplier);
}
else
{
// TODO: Investigate cases where it makes sense to inline non-foldable switches
multiplier = 0;
JITDUMP("\nInline candidate has %d switches. Multiplier limited to %g.", m_Switch, multiplier);
}
}

if (m_HasProfile)
{
// There are cases when Profile Data can be misleading or polluted:
Expand Down Expand Up @@ -1738,6 +1766,8 @@ void ExtendedDefaultPolicy::OnDumpXml(FILE* file, unsigned indent) const
XATTR_I4(m_FoldableExpr)
XATTR_I4(m_FoldableExprUn)
XATTR_I4(m_FoldableBranch)
XATTR_I4(m_FoldableSwitch)
XATTR_I4(m_Switch)
XATTR_I4(m_DivByCns)
XATTR_B(m_ReturnsStructByValue)
XATTR_B(m_IsFromValueClass)
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/inlinepolicy.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ class ExtendedDefaultPolicy : public DefaultPolicy
, m_FoldableExpr(0)
, m_FoldableExprUn(0)
, m_FoldableBranch(0)
, m_FoldableSwitch(0)
, m_Switch(0)
, m_DivByCns(0)
, m_ReturnsStructByValue(false)
, m_IsFromValueClass(false)
Expand Down Expand Up @@ -252,6 +254,8 @@ class ExtendedDefaultPolicy : public DefaultPolicy
unsigned m_FoldableExpr;
unsigned m_FoldableExprUn;
unsigned m_FoldableBranch;
unsigned m_FoldableSwitch;
unsigned m_Switch;
unsigned m_DivByCns;
bool m_ReturnsStructByValue : 1;
bool m_IsFromValueClass : 1;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ CONFIG_STRING(JitInlineReplayFile, W("JitInlineReplayFile"))
// Extended version of DefaultPolicy that includes a more precise IL scan,
// relies on PGO if it exists and generally is more aggressive.
CONFIG_INTEGER(JitExtDefaultPolicy, W("JitExtDefaultPolicy"), 1)
CONFIG_INTEGER(JitExtDefaultPolicyMaxIL, W("JitExtDefaultPolicyMaxIL"), 0x64)
CONFIG_INTEGER(JitExtDefaultPolicyMaxIL, W("JitExtDefaultPolicyMaxIL"), 0x400)
CONFIG_INTEGER(JitExtDefaultPolicyMaxBB, W("JitExtDefaultPolicyMaxBB"), 7)

// Inliner uses the following formula for PGO-driven decisions:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Globalization;
using System.IO;
using System.Reflection;
using System.Runtime.CompilerServices;

namespace System.Linq.Expressions
{
Expand Down Expand Up @@ -168,6 +169,7 @@ private void Out(string s, Flow after)
Out(Flow.None, s, after);
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void Out(Flow before, string s, Flow after)
{
switch (GetFlow(before))
Expand Down