From 14120d399fa46844bd60c66b5b4bdb4c5ed6435d Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 24 Feb 2016 16:14:32 -0800 Subject: [PATCH] InlineRefactoring: capturing bits of legacy policy Rework the logic for force inline, basic block count, il size, and maxstack so that the policy decides when these values should inhibit inlining. --- src/jit/compiler.cpp | 13 ++++++---- src/jit/compiler.h | 11 +++++---- src/jit/flowgraph.cpp | 9 ++++--- src/jit/importer.cpp | 26 ++++++-------------- src/jit/inline.def | 5 ++-- src/jit/inlinepolicy.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++++--- src/jit/inlinepolicy.h | 12 ++++++++- 7 files changed, 103 insertions(+), 37 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 0af9efc..5cca548 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -4962,13 +4962,16 @@ int Compiler::compCompileHelper (CORINFO_MODULE_HANDLE clas compGenTreeID = 0; #endif - if (compIsForInlining() && - (fgBBcount > 5) && - !forceInline) + if (compIsForInlining()) { - compInlineResult->note(InlineObservation::CALLEE_TOO_MANY_BASIC_BLOCKS); + if (forceInline) + { + compInlineResult->noteCandidate(InlineObservation::CALLEE_IS_FORCE_INLINE); + } + + compInlineResult->noteInt(InlineObservation::CALLEE_NUMBER_OF_BASIC_BLOCKS, fgBBcount); - if (compInlineResult->isFailure()) + if (compInlineResult->isFailure()) { goto _Next; } diff --git a/src/jit/compiler.h b/src/jit/compiler.h index bb302ee..0add1fd 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -2509,6 +2509,9 @@ public : InlineInfo * impInlineInfo; + // Get the maximum IL size allowed for an inline + unsigned getImpInlineSize() const { return impInlineSize; } + // The Compiler* that is the root of the inlining tree of which "this" is a member. Compiler* impInlineRoot(); @@ -2583,8 +2586,10 @@ protected : //-------------------- Stack manipulation --------------------------------- unsigned impStkSize; // Size of the full stack - StackEntry impSmallStack[16]; // Use this array is possible +#define SMALL_STACK_SIZE 16 // number of elements in impSmallStack + + StackEntry impSmallStack[SMALL_STACK_SIZE]; // Use this array if possible struct SavedStack // used to save/restore stack contents. { @@ -3077,10 +3082,6 @@ private: static unsigned jitIciStmtIsTheLastInBB; static unsigned jitInlineeContainsOnlyOneBB; - #define INLINER_FAILED "\nINLINER FAILED: " - #define INLINER_WARNING "\nINLINER WARNING: " - #define INLINER_INFO "\nINLINER INFO: " - #endif // defined(DEBUG) || MEASURE_INLINING #ifdef DEBUG diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index 0dc9fe6..eca2e19 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -4423,8 +4423,11 @@ DECODE_OPCODE: if (compIsForInlining()) { - compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_SWITCH); - return; + compInlineResult->note(InlineObservation::CALLEE_HAS_SWITCH); + if (compInlineResult->isFailure()) + { + return; + } } // Make sure we don't go past the end reading the number of cases @@ -21919,7 +21922,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, JITLOG_THIS(pParam->pThis, (LL_INFO100000, - INLINER_INFO "inlineInfo.tokenLookupContextHandle for %s set to 0x%p:\n", + "INLINER: inlineInfo.tokenLookupContextHandle for %s set to 0x%p:\n", pParam->pThis->eeGetMethodFullName(pParam->fncHandle), pParam->pThis->dspPtr(pParam->inlineInfo->tokenLookupContextHandle))); diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index e4e5e0c..dc79b76 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -15818,6 +15818,11 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, return; } + if (forceInline) + { + inlineResult->noteCandidate(InlineObservation::CALLEE_IS_FORCE_INLINE); + } + // Reject if it has too many locals. // This is currently an implementation limit due to fixed-size arrays in the // inline info, rather than a performance heuristic. @@ -15842,36 +15847,21 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, return; } - if (forceInline) - { - // This looks a bit redundant; it's because we haven't yet - // extracted policy from observation.... - inlineResult->note(InlineObservation::CALLEE_HAS_FORCE_INLINE); - inlineResult->noteCandidate(InlineObservation::CALLEE_HAS_FORCE_INLINE); - return; - } - // Reject large functions inlineResult->noteInt(InlineObservation::CALLEE_NUMBER_OF_IL_BYTES, codeSize); - if (codeSize > impInlineSize) + if (inlineResult->isFailure()) { - inlineResult->note(InlineObservation::CALLEE_TOO_MUCH_IL); - - if (inlineResult->isFailure()) - { - return; - } + return; } // Make sure maxstack is not too big inlineResult->noteInt(InlineObservation::CALLEE_MAXSTACK, methInfo->maxStack); - if (methInfo->maxStack > sizeof(impSmallStack)/sizeof(impSmallStack[0])) + if (inlineResult->isFailure()) { - inlineResult->noteFatal(InlineObservation::CALLEE_MAXSTACK_TOO_BIG); return; } diff --git a/src/jit/inline.def b/src/jit/inline.def index fa2ab24..d602af8 100644 --- a/src/jit/inline.def +++ b/src/jit/inline.def @@ -41,7 +41,6 @@ INLINE_OBSERVATION(HAS_NATIVE_VARARGS, bool, "native varargs", INLINE_OBSERVATION(HAS_NO_BODY, bool, "has no body", FATAL, CALLEE) INLINE_OBSERVATION(HAS_NULL_FOR_LDELEM, bool, "has null pointer for ldelem", FATAL, CALLEE) INLINE_OBSERVATION(HAS_PINNED_LOCALS, bool, "has pinned locals", FATAL, CALLEE) -INLINE_OBSERVATION(HAS_SWITCH, bool, "has switch", FATAL, CALLEE) INLINE_OBSERVATION(IS_ARRAY_METHOD, bool, "is array method", FATAL, CALLEE) INLINE_OBSERVATION(IS_GENERIC_VIRTUAL, bool, "generic virtual", FATAL, CALLEE) INLINE_OBSERVATION(IS_JIT_NOINLINE, bool, "noinline per JitNoinline", FATAL, CALLEE) @@ -66,6 +65,7 @@ INLINE_OBSERVATION(UNSUPPORTED_OPCODE, bool, "unsupported opcode", // ------ Callee Performance ------- +INLINE_OBSERVATION(HAS_SWITCH, bool, "has switch", PERFORMANCE, CALLEE) INLINE_OBSERVATION(LDFLD_STATIC_VALUECLASS, bool, "ldsfld of value class", PERFORMANCE, CALLEE) INLINE_OBSERVATION(TOO_MANY_BASIC_BLOCKS, bool, "too many basic blocks", PERFORMANCE, CALLEE) INLINE_OBSERVATION(TOO_MUCH_IL, bool, "too many il bytes", PERFORMANCE, CALLEE) @@ -75,10 +75,11 @@ INLINE_OBSERVATION(TOO_MUCH_IL, bool, "too many il bytes", INLINE_OBSERVATION(BELOW_ALWAYS_INLINE_SIZE, bool, "below ALWAYS_INLINE size", INFORMATION, CALLEE) INLINE_OBSERVATION(CAN_INLINE_IL, bool, "IL passes basic checks", INFORMATION, CALLEE) INLINE_OBSERVATION(CHECK_CAN_INLINE_IL, bool, "IL passes detailed checks", INFORMATION, CALLEE) -INLINE_OBSERVATION(HAS_FORCE_INLINE, bool, "aggressive inline attribute", INFORMATION, CALLEE) +INLINE_OBSERVATION(IS_FORCE_INLINE, bool, "aggressive inline attribute", INFORMATION, CALLEE) INLINE_OBSERVATION(MAXSTACK, int, "maxstack", INFORMATION, CALLEE) INLINE_OBSERVATION(NATIVE_SIZE_ESTIMATE, double, "native size estimate", INFORMATION, CALLEE) INLINE_OBSERVATION(NUMBER_OF_ARGUMENTS, int, "number of arguments", INFORMATION, CALLEE) +INLINE_OBSERVATION(NUMBER_OF_BASIC_BLOCKS, int, "number of basic blocks", INFORMATION, CALLEE) INLINE_OBSERVATION(NUMBER_OF_IL_BYTES, int, "number of bytes of IL", INFORMATION, CALLEE) INLINE_OBSERVATION(NUMBER_OF_LOCALS, int, "number of locals", INFORMATION, CALLEE) diff --git a/src/jit/inlinepolicy.cpp b/src/jit/inlinepolicy.cpp index 829a724..0be86bf 100644 --- a/src/jit/inlinepolicy.cpp +++ b/src/jit/inlinepolicy.cpp @@ -25,7 +25,7 @@ InlinePolicy* InlinePolicy::getPolicy(Compiler* compiler) { // For now, always create a Legacy policy. - InlinePolicy* policy = new (compiler, CMK_Inlining) LegacyPolicy(); + InlinePolicy* policy = new (compiler, CMK_Inlining) LegacyPolicy(compiler); return policy; } @@ -44,6 +44,18 @@ void LegacyPolicy::noteCandidate(InlineObservation obs) InlineImpact impact = inlGetImpact(obs); assert(impact == InlineImpact::INFORMATION); + switch (obs) + { + case InlineObservation::CALLEE_IS_FORCE_INLINE: + { + inlIsForceInline = true; + break; + } + + default: + break; + } + // Update the status setCommon(InlineDecision::CANDIDATE, obs); } @@ -71,6 +83,7 @@ void LegacyPolicy::note(InlineObservation obs) // As a safeguard, all fatal impact must be // reported via noteFatal. assert(impact != InlineImpact::FATAL); + noteInternal(obs, impact); } @@ -101,8 +114,53 @@ void LegacyPolicy::noteFatal(InlineObservation obs) void LegacyPolicy::noteInt(InlineObservation obs, int value) { - (void) value; - note(obs); + switch (obs) + { + case InlineObservation::CALLEE_MAXSTACK: + { + unsigned calleeMaxStack = static_cast(value); + + if (calleeMaxStack > SMALL_STACK_SIZE) + { + setNever(InlineObservation::CALLEE_MAXSTACK_TOO_BIG); + } + + break; + } + + case InlineObservation::CALLEE_NUMBER_OF_BASIC_BLOCKS: + { + assert(value != 0); + + unsigned basicBlockCount = static_cast(value); + + if (!inlIsForceInline && (basicBlockCount > MAX_BASIC_BLOCKS)) + { + setNever(InlineObservation::CALLEE_TOO_MANY_BASIC_BLOCKS); + } + + break; + } + + case InlineObservation::CALLEE_NUMBER_OF_IL_BYTES: + { + assert(value != 0); + + unsigned ilByteSize = static_cast(value); + + if (ilByteSize > inlCompiler->getImpInlineSize()) + { + setNever(InlineObservation::CALLEE_TOO_MUCH_IL); + } + + break; + } + + default: + // Ignore all other information + note(obs); + break; + } } //------------------------------------------------------------------------ diff --git a/src/jit/inlinepolicy.h b/src/jit/inlinepolicy.h index 3f883b2..7a4c6fb 100644 --- a/src/jit/inlinepolicy.h +++ b/src/jit/inlinepolicy.h @@ -24,8 +24,11 @@ class LegacyPolicy : public InlinePolicy { public: - LegacyPolicy() + // Construct a LegacyPolicy + LegacyPolicy(Compiler* compiler) : InlinePolicy() + , inlCompiler(compiler) + , inlIsForceInline(false) { // empty } @@ -52,6 +55,13 @@ private: void setFailure(InlineObservation obs); void setNever(InlineObservation obs); void setCommon(InlineDecision decision, InlineObservation obs); + + // Constants + const unsigned MAX_BASIC_BLOCKS = 5; + + // Data members + Compiler* inlCompiler; + bool inlIsForceInline; }; // -- 2.7.4