From 0e0229a5c5595bd59779f5a1b0213e1f9cfb6972 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 8 Feb 2016 11:08:57 -0800 Subject: [PATCH] Inline refactoring: start untangling observation and policy Create set of observations that can be made during inlining. These are implemented as a set of static tables plus some helper methods. Each observation has an enum name, a type, a description string, an impact, and a target. For now most observations are about blocking issues and are classified as having FATAL impact. There are a handful of INFORMATIONAL and PERFORMANCE observations but they're not widely used yet. This change also updates the bulk of the jit code to report observations to the JitInlineResult instead of directly requesting changes to the JitInlineResult state. Over on the JitInlineResult side, the current legacy policy is implemented and fails fast if any blocking observation is made. For now, any any FATAL impact observation must be made via `noteFatal`, and all other observations be made via `note`. As with the previous refactorings, this change tries not to alter any code generation. There are a few cases where observations that are made solely about the callee are now targeted that way instead of being targeted at callsites. For instance a method that is marked by COMPLUS_JitNoInline will never be inlined. This can sometimes lead to localized code diffs, since the jit creates slightly different IR for a call to an inline candidate than a call to a non-candidate, and is not always able to undo this later if inlining fails. However the number of diffs should be small. Will verify diffs further before merging. There are no inlining changes crossgenning mscorlib. Some of the message strings associated with inlining failures have changed. The messages use `caller` and `callee` to describe the two methods involved, and `callsite` for the instance in question, and deprecate `inlinee`. These message strings can be seen in the jit dumps and logs and are reported back to the VM where they presumably make their way into other diagnostic reporting streams. Subsequent work will re-examine the FATAL observations and likely reclassify a number of them to have less dramatic immediate impact. Subsequent work will also begin extracting the policy into a separate class to lay the groundwork for supporting alternate policies while still being able to fall back to the legacy policy. Commit migrated from https://github.com/dotnet/coreclr/commit/aafcd1a1793e9d1853ef7aa1445447fa11f26a85 --- src/coreclr/src/jit/CMakeLists.txt | 1 + src/coreclr/src/jit/compiler.cpp | 12 +- src/coreclr/src/jit/compiler.h | 114 ++++++++++---- src/coreclr/src/jit/compiler.hpp | 3 +- src/coreclr/src/jit/flowgraph.cpp | 36 ++--- src/coreclr/src/jit/importer.cpp | 262 +++++++++++++++++-------------- src/coreclr/src/jit/inline.cpp | 154 ++++++++++++++++++ src/coreclr/src/jit/inline.def | 153 ++++++++++++++++++ src/coreclr/src/jit/inline.h | 72 +++++++++ src/coreclr/src/jit/jit.settings.targets | 1 + src/coreclr/src/jit/jitpch.h | 2 + src/coreclr/src/jit/morph.cpp | 17 +- 12 files changed, 646 insertions(+), 181 deletions(-) create mode 100644 src/coreclr/src/jit/inline.cpp create mode 100644 src/coreclr/src/jit/inline.def create mode 100644 src/coreclr/src/jit/inline.h diff --git a/src/coreclr/src/jit/CMakeLists.txt b/src/coreclr/src/jit/CMakeLists.txt index a5f777e..e8c7204 100644 --- a/src/coreclr/src/jit/CMakeLists.txt +++ b/src/coreclr/src/jit/CMakeLists.txt @@ -61,6 +61,7 @@ set( JIT_SOURCES loopcloning.cpp lower.cpp lsra.cpp + inline.cpp ) if(CLR_CMAKE_PLATFORM_ARCH_AMD64) diff --git a/src/coreclr/src/jit/compiler.cpp b/src/coreclr/src/jit/compiler.cpp index 6b8e7bb..f41ff4c 100644 --- a/src/coreclr/src/jit/compiler.cpp +++ b/src/coreclr/src/jit/compiler.cpp @@ -4362,7 +4362,7 @@ int Compiler::compCompile(CORINFO_METHOD_HANDLE methodHnd, { if (compIsForInlining()) { - compInlineResult->setNever("Inlinee marked as skipped"); + compInlineResult->noteFatal(InlineObservation::CALLEE_MARKED_AS_SKIPPED); } return CORJIT_SKIPPED; } @@ -5079,8 +5079,12 @@ int Compiler::compCompileHelper (CORINFO_MODULE_HANDLE clas (fgBBcount > 5) && !forceInline) { - compInlineResult->setNever("Too many basic blocks in the inlinee"); - goto _Next; + compInlineResult->note(InlineObservation::CALLEE_TOO_MANY_BASIC_BLOCKS); + + if (compInlineResult->isFailure()) + { + goto _Next; + } } #ifdef DEBUG @@ -5760,7 +5764,7 @@ START: { // Note that we failed to compile the inlinee, and that // there's no point trying to inline it again anywhere else. - inlineInfo->inlineResult->setNever("Error compiling inlinee"); + inlineInfo->inlineResult->noteFatal(InlineObservation::CALLEE_COMPILATION_ERROR); } param.result = __errc; } diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index f1127b7..3a2a819 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -25,6 +25,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX #include "jit.h" #include "opcode.h" #include "block.h" +#include "inline.h" #include "jiteh.h" #include "instr.h" #include "regalloc.h" @@ -857,19 +858,6 @@ const unsigned int MAX_INL_ARGS = 10; // does not include obj pointer const unsigned int MAX_INL_LCLS = 8; #endif // LEGACY_BACKEND -// InlineDecision describes the various states the jit goes through when -// evaluating an inline candidate. It is distinct from CorInfoInline -// because it must capture interal states that don't get reported back -// to the runtime. - -enum class InlineDecision { - UNDECIDED = 1, - CANDIDATE = 2, - SUCCESS = 3, - FAILURE = 4, - NEVER = 5 -}; - // JitInlineResult encapsulates what is known about a particular // inline candiate. @@ -1017,29 +1005,60 @@ public: assert(!isFailure()); inlDecision = InlineDecision::SUCCESS; } - - // setFailure means this particular instance can't be inlined. - // It can override setCandidate, but not setSuccess - void setFailure(const char* reason) + + // Make an observation, and update internal state appropriately. + // + // Caller is expected to call isFailure after this to see whether + // more observation is desired. + void note(InlineObservation obs) { - assert(!isSuccess()); - setCommon(InlineDecision::FAILURE, reason); + // Check the impact + InlineImpact impact = inlGetImpact(obs); + + // As a safeguard, all fatal impact must be + // reported via noteFatal. + assert(impact != InlineImpact::FATAL); + noteInternal(obs, impact); } - - // setNever means this callee can never be inlined anywhere. - // It can override setCandidate, but not setSuccess - void setNever(const char* reason) + + // Make an observation where caller knows for certain that this + // inline cannot happen, and so there's no point in any further + // scrutiny of this inline. Update internal state to mark the + // inline result as a failure. + void noteFatal(InlineObservation obs) { - assert(!isSuccess()); - setCommon(InlineDecision::NEVER, reason); + // Check the impact + InlineImpact impact = inlGetImpact(obs); + + // As a safeguard, all fatal impact must be + // reported via noteFatal. + assert(impact == InlineImpact::FATAL); + noteInternal(obs, impact); + assert(isFailure()); + } + + // Ignore values for now + void noteInt(InlineObservation obs, int value) + { + (void) value; + note(obs); + } + + // Ignore values for now + void noteDouble(InlineObservation obs, double value) + { + (void) value; + note(obs); } - // Report/log/dump decision as appropriate + // Ensure result is appropriately reported when the result goes + // out of scope. ~JitInlineResult() { report(); } - + + // The reason for this particular result const char * reason() const { return inlReason; } // setReported indicates that this particular result doesn't need @@ -1053,6 +1072,46 @@ private: JitInlineResult(const JitInlineResult&) = delete; JitInlineResult operator=(const JitInlineResult&) = delete; + // Handle implications of an inline observation + void noteInternal(InlineObservation obs, InlineImpact impact) + { + // Ignore INFORMATION for now, since policy + // is still embedded at the observation sites. + if (impact == InlineImpact::INFORMATION) + { + return; + } + + InlineTarget target = inlGetTarget(obs); + const char* reason = inlGetDescriptionString(obs); + + if (target == InlineTarget::CALLEE) + { + this->setNever(reason); + } + else + { + this->setFailure(reason); + } + } + + // setFailure means this particular instance can't be inlined. + // It can override setCandidate, but not setSuccess + void setFailure(const char* reason) + { + assert(!isSuccess()); + setCommon(InlineDecision::FAILURE, reason); + } + + // setNever means this callee can never be inlined anywhere. + // It can override setCandidate, but not setSuccess + void setNever(const char* reason) + { + assert(!isSuccess()); + setCommon(InlineDecision::NEVER, reason); + } + + // Helper for setting decision and reason void setCommon(InlineDecision decision, const char* reason) { assert(reason != nullptr); @@ -1061,6 +1120,7 @@ private: inlReason = reason; } + // Report/log/dump decision as appropriate void report(); Compiler* inlCompiler; diff --git a/src/coreclr/src/jit/compiler.hpp b/src/coreclr/src/jit/compiler.hpp index 0f398a6..b5543e6 100644 --- a/src/coreclr/src/jit/compiler.hpp +++ b/src/coreclr/src/jit/compiler.hpp @@ -1583,8 +1583,7 @@ inline unsigned Compiler::lvaGrabTemp(bool shortLifetime if (pComp->lvaHaveManyLocals()) { // Don't create more LclVar with inlining - JITLOG((LL_INFO1000000, INLINER_FAILED "Inlining requires new LclVars and we already have too many locals.")); - compInlineResult->setFailure("Inlining requires new LclVars and we already have too many locals."); + compInlineResult->noteFatal(InlineObservation::CALLSITE_TOO_MANY_LOCALS); } unsigned tmpNum = pComp->lvaGrabTemp(shortLifetime DEBUGARG(reason)); diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index e95a8db..9c19590 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -4438,7 +4438,7 @@ DECODE_OPCODE: { if (stateNetCFQuirks >= 0) { - compInlineResult->setNever("Windows Phone OS 7 compatibility - Inlinee contains control flow"); + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_CONTROL_FLOW); return; } } @@ -4484,13 +4484,13 @@ DECODE_OPCODE: if (stateNetCFQuirks >= 0) { - compInlineResult->setNever("Windows Phone OS 7 compatibility - Inlinee contains control flow"); + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_CONTROL_FLOW); return; } #endif // FEATURE_LEGACYNETCF - compInlineResult->setNever("Inlinee contains SWITCH instruction"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_SWITCH); return; } @@ -4554,7 +4554,7 @@ DECODE_OPCODE: { if (stateNetCFQuirks >= 0) { - compInlineResult->setNever("Windows Phone OS 7 compatibility - Inlinee contains prefix"); + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_PREFIX); return; } } @@ -4580,7 +4580,7 @@ DECODE_OPCODE: { if (stateNetCFQuirks >= 0) { - compInlineResult->setNever("Windows Phone OS 7 compatibility - Inlinee contains throw"); + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_THROW); return; } } @@ -4631,15 +4631,7 @@ DECODE_OPCODE: //Consider making this only for not force inline. if (compIsForInlining()) { - const char* message; -#ifdef DEBUG - message = (char*)compAllocator->nraAlloc(128); - sprintf((char*)message, "Unsupported opcode for inlining: %s\n", - opcodeNames[opcode]); -#else - message = "Unsupported opcode for inlining"; -#endif - compInlineResult->setNever(message); + compInlineResult->noteFatal(InlineObservation::CALLEE_UNSUPPORTED_OPCODE); return; } break; @@ -4764,7 +4756,7 @@ ARG_PUSH: stateNetCFQuirks++; if (varNum != expectedVarNum) { - compInlineResult->setNever("Windows Phone OS 7 compatibility - out of order ldarg"); + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_LDARG_ORDER); return; } } @@ -4783,7 +4775,7 @@ ADDR_TAKEN: { if (stateNetCFQuirks >= 0) { - compInlineResult->setNever("Windows Phone OS 7 compatibility - address taken"); + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_ADDRESS_TAKEN); return; } } @@ -4910,7 +4902,7 @@ ARG_WRITE: /* The inliner keeps the args as trees and clones them. Storing the arguments breaks that * simplification. To allow this, flag the argument as written to and spill it before * inlining. That way the STARG in the inlinee is trivial. */ - compInlineResult->setNever("Inlinee writes to an argument"); + compInlineResult->noteFatal(InlineObservation::CALLEE_STORES_TO_ARGUMENT); return; } else @@ -21916,7 +21908,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, printf("Recursive or deep inline recursion detected. Will not expand this INLINECANDIDATE \n"); } #endif // DEBUG - inlineResult->setFailure("Recursive or deep inline"); + inlineResult->noteFatal(InlineObservation::CALLSITE_IS_RECURSIVE_OR_DEEP); return; } @@ -21991,7 +21983,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, if (result != CORJIT_OK) { - pParam->inlineInfo->inlineResult->setFailure("Error invoking the compiler for the inlinee"); + pParam->inlineInfo->inlineResult->noteFatal(InlineObservation::CALLSITE_COMPILATION_FAILURE); } } } @@ -22004,7 +21996,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, eeGetMethodFullName(fncHandle)); } #endif // DEBUG - inlineResult->setFailure("Exception invoking the compiler for the inlinee"); + inlineResult->noteFatal(InlineObservation::CALLSITE_COMPILATION_ERROR); } endErrorTrap(); @@ -22038,7 +22030,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, eeGetMethodFullName(fncHandle)); } #endif // DEBUG - inlineResult->setNever("inlinee did not contain a return expression"); + inlineResult->noteFatal(InlineObservation::CALLEE_LACKS_RETURN); return; } @@ -22049,7 +22041,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, if (!(info.compCompHnd->initClass(NULL /* field */, fncHandle /* method */, inlineCandidateInfo->exactContextHnd /* context */) & CORINFO_INITCLASS_INITIALIZED)) { - inlineResult->setNever("Failed class init side-effect"); + inlineResult->noteFatal(InlineObservation::CALLEE_CLASS_INIT_FAILURE); return; } } diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 1a40e6e..c211c35 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -1599,7 +1599,7 @@ GenTreePtr Compiler::impLookupToTree(CORINFO_LOOKUP *pLookup, unsigned { // Don't import runtime lookups when inlining // Inlining has to be aborted in such a case - compInlineResult->setFailure("Cannot inline generic dictionary lookup"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_GENERIC_DICTIONARY_LOOKUP); return nullptr; } else @@ -1679,7 +1679,7 @@ GenTreePtr Compiler::impMethodPointer(CORINFO_RESOLVED_TOKEN * pResolvedToken, C { // Don't import runtime lookups when inlining // Inlining has to be aborted in such a case - compInlineResult->setFailure("Cannot inline generic dictionary lookup"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_GENERIC_DICTIONARY_LOOKUP); return nullptr; } @@ -5673,10 +5673,11 @@ var_types Compiler::impImportCall (OPCODE opcode, if (compIsForInlining()) { + /* Does this call site have security boundary restrictions? */ + if (impInlineInfo->inlineCandidateInfo->dwRestrictions & INLINE_RESPECT_BOUNDARY) { - //Check to see if this call violates the boundary. - compInlineResult->setFailure("Inline failed due to cross security boundary call"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_CROSS_BOUNDARY_SECURITY); return callRetTyp; } @@ -5684,7 +5685,7 @@ var_types Compiler::impImportCall (OPCODE opcode, if (mflags & CORINFO_FLG_SECURITYCHECK) { - compInlineResult->setNever("Inlinee needs own frame for security object"); + compInlineResult->noteFatal(InlineObservation::CALLEE_NEEDS_SECURITY_CHECK); return callRetTyp; } @@ -5692,7 +5693,7 @@ var_types Compiler::impImportCall (OPCODE opcode, if (mflags & CORINFO_FLG_DONT_INLINE_CALLER) { - compInlineResult->setNever("Inlinee calls a method that uses StackCrawlMark"); + compInlineResult->noteFatal(InlineObservation::CALLEE_STACK_CRAWL_MARK); return callRetTyp; } @@ -5700,29 +5701,28 @@ var_types Compiler::impImportCall (OPCODE opcode, if (mflags & CORINFO_FLG_DELEGATE_INVOKE) { - compInlineResult->setNever("Inlinee uses delegate invoke"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_DELEGATE_INVOKE); return callRetTyp; } /* For now ignore varargs */ if ((sig->callConv & CORINFO_CALLCONV_MASK) == CORINFO_CALLCONV_NATIVEVARARG) { - compInlineResult->setNever("Native VarArgs not supported in inlinee"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_NATIVE_VARARGS); return callRetTyp; } if ((sig->callConv & CORINFO_CALLCONV_MASK) == CORINFO_CALLCONV_VARARG) { - compInlineResult->setNever("VarArgs not supported in inlinee"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_MANAGED_VARARGS); return callRetTyp; } if ((mflags & CORINFO_FLG_VIRTUAL) && (sig->sigInst.methInstCount != 0) && (opcode == CEE_CALLVIRT)) { - compInlineResult->setNever("inlining virtual calls on generic functions not supported"); + compInlineResult->noteFatal(InlineObservation::CALLEE_IS_GENERIC_VIRTUAL); return callRetTyp; } - } clsHnd = pResolvedToken->hClass; @@ -5849,7 +5849,7 @@ var_types Compiler::impImportCall (OPCODE opcode, * always have a handle lookup. These lookups are safe intra-module, but we're just * failing here. */ - compInlineResult->setFailure("Cannot inline complicated handle access"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_HAS_COMPLEX_HANDLE); return callRetTyp; } @@ -5926,7 +5926,7 @@ var_types Compiler::impImportCall (OPCODE opcode, { if (compIsForInlining()) { - compInlineResult->setFailure("Calling through LDVIRTFTN"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_HAS_CALL_VIA_LDVIRTFTN); return callRetTyp; } @@ -6203,7 +6203,7 @@ var_types Compiler::impImportCall (OPCODE opcode, // Cannot handle this if the method being imported is an inlinee by itself. // Because inlinee method does not have its own frame. - compInlineResult->setNever("Inlinee needs own frame for security object"); + compInlineResult->noteFatal(InlineObservation::CALLEE_NEEDS_SECURITY_CHECK); return callRetTyp; } else @@ -6261,7 +6261,7 @@ var_types Compiler::impImportCall (OPCODE opcode, // so instead we fallback to JIT. if (compIsForInlining()) { - compInlineResult->setFailure("Cannot embed PInvoke cookie across modules"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_CANT_EMBED_PINVOKE_COOKIE); } else { @@ -6312,7 +6312,7 @@ var_types Compiler::impImportCall (OPCODE opcode, void * varCookie, *pVarCookie; if (!info.compCompHnd->canGetVarArgsHandle(sig)) { - compInlineResult->setFailure("Cannot embed Var Args handle across modules"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_CANT_EMBED_VARARGS_COOKIE); return callRetTyp; } @@ -6392,7 +6392,7 @@ var_types Compiler::impImportCall (OPCODE opcode, if (compIsForInlining() && (clsFlags & CORINFO_FLG_ARRAY) != 0) { - compInlineResult->setNever("Array method"); + compInlineResult->noteFatal(InlineObservation::CALLEE_IS_ARRAY_METHOD); return callRetTyp; } @@ -8872,7 +8872,7 @@ PUSH_I4CON: { if (impInlineInfo->inlineCandidateInfo->dwRestrictions & INLINE_NO_CALLEE_LDSTR) { - compInlineResult->setFailure("Cross assembly inline failed due to NoStringInterning"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_HAS_LDSTR_RESTRICTION); return; } } @@ -9219,7 +9219,7 @@ _PopValue: op1 = impInlineFetchArg(lclNum, impInlineInfo->inlArgInfo, impInlineInfo->lclVarInfo); if (op1->gtOper != GT_LCL_VAR) { - compInlineResult->setFailure("Inline failed due to LDARGA on something other than a variable"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_LDARGA_NOT_LOCAL_VAR); return; } @@ -9300,7 +9300,7 @@ _PUSH_ADRVAR: if (compIsForInlining()) { assert(!"Shouldn't have exception handlers in the inliner!"); - compInlineResult->setNever("Unexpected endfinally in inlinee"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_ENDFINALLY); return; } @@ -9320,7 +9320,7 @@ _PUSH_ADRVAR: if (compIsForInlining()) { assert(!"Shouldn't have exception handlers in the inliner!"); - compInlineResult->setNever("Unexpected endfilter in inlinee"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_ENDFILTER); return; } @@ -9620,7 +9620,7 @@ ARR_LD_POST_VERIFY: { if (op1->gtOper == GT_CNS_INT) { - compInlineResult->setNever("Inlinee contains NULL pointer for LDELEM"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_NULL_FOR_LDELEM); return; } } @@ -10137,7 +10137,7 @@ CEE_SH_OP2: if (compIsForInlining()) { - compInlineResult->setNever("Inlinee contains LEAVE instruction"); + compInlineResult->noteFatal(InlineObservation::CALLEE_HAS_LEAVE); return; } @@ -11066,7 +11066,7 @@ DO_LDFTN: { if (mflags & (CORINFO_FLG_FINAL|CORINFO_FLG_STATIC) || !(mflags & CORINFO_FLG_VIRTUAL)) { - compInlineResult->setFailure("Inline failed due to ldvirtftn on non-virtual function"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_LDVIRTFN_ON_NON_VIRTUAL); return; } } @@ -11213,7 +11213,7 @@ DO_LDFTN: if (impInlineInfo->inlineCandidateInfo->dwRestrictions & INLINE_RESPECT_BOUNDARY) { //Check to see if this call violates the boundary. - compInlineResult->setFailure("Inline failed due to cross security boundary call"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_CROSS_BOUNDARY_SECURITY); return; } } @@ -11279,7 +11279,7 @@ DO_LDFTN: { if (varTypeIsComposite(JITtype2varType(callInfo.sig.retType)) && callInfo.sig.retTypeClass == NULL) { - compInlineResult->setNever("return type is composite"); + compInlineResult->noteFatal(InlineObservation::CALLEE_RETURN_TYPE_IS_COMPOSITE); return; } } @@ -11418,7 +11418,7 @@ DO_LDFTN: //CALLI doesn't have a method handle, so assume the worst. if (impInlineInfo->inlineCandidateInfo->dwRestrictions & INLINE_RESPECT_BOUNDARY) { - compInlineResult->setFailure("Inline failed due to cross assembly call to a boundary method"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_CROSS_BOUNDARY_CALLI); return; } } @@ -11667,18 +11667,16 @@ DO_LDFTN: { case CORINFO_FIELD_INSTANCE_HELPER: case CORINFO_FIELD_INSTANCE_ADDR_HELPER: - case CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER: case CORINFO_FIELD_STATIC_ADDR_HELPER: case CORINFO_FIELD_STATIC_TLS: - /* We may be able to inlining the field accessors in specific instantiations of generic methods */ - if (fieldInfo.fieldAccessor == CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER) - { - compInlineResult->setFailure("Inlinee requires field access helper (shared generic static"); - } - else - { - compInlineResult->setNever("Inlinee requires field access helper"); - } + + compInlineResult->noteFatal(InlineObservation::CALLEE_LDFLD_NEEDS_HELPER); + return; + + case CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER: + + /* We may be able to inline the field accessors in specific instantiations of generic methods */ + compInlineResult->noteFatal(InlineObservation::CALLSITE_LDFLD_NEEDS_HELPER); return; default: @@ -11695,8 +11693,12 @@ DO_LDFTN: { // Loading a static valuetype field usually will cause a JitHelper to be called // for the static base. This will bloat the code. - compInlineResult->setNever("Inlinee loads static field of valueclass"); - return; + compInlineResult->note(InlineObservation::CALLEE_LDFLD_STATIC_VALUECLASS); + + if (compInlineResult->isFailure()) + { + return; + } } } } @@ -12009,18 +12011,16 @@ FIELD_DONE: { case CORINFO_FIELD_INSTANCE_HELPER: case CORINFO_FIELD_INSTANCE_ADDR_HELPER: - case CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER: case CORINFO_FIELD_STATIC_ADDR_HELPER: case CORINFO_FIELD_STATIC_TLS: - /* We may be able to inlining the field accessors in specific instantiations of generic methods */ - if (fieldInfo.fieldAccessor == CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER) - { - compInlineResult->setFailure("Inlinee requires field access helper (static shared generic)"); - } - else - { - compInlineResult->setNever("Inlinee requires field access helper"); - } + + compInlineResult->noteFatal(InlineObservation::CALLEE_STFLD_NEEDS_HELPER); + return; + + case CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER: + + /* We may be able to inline the field accessors in specific instantiations of generic methods */ + compInlineResult->noteFatal(InlineObservation::CALLSITE_STFLD_NEEDS_HELPER); return; default: @@ -12907,7 +12907,7 @@ FIELD_DONE: { /* if not, just don't inline the method */ - compInlineResult->setNever("Reached CEE_THROW with too many things on the stack."); + compInlineResult->noteFatal(InlineObservation::CALLEE_THROW_WITH_INVALID_STACK); return; } @@ -12921,7 +12921,7 @@ FIELD_DONE: if (seenConditionalJump && (impInlineInfo->inlineCandidateInfo->fncRetType != TYP_VOID)) { - compInlineResult->setFailure("No inlining for THROW within a non-void conditional"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_CONDITIONAL_THROW); return; } @@ -13393,7 +13393,7 @@ void Compiler::impLoadArg(unsigned ilArgNum, IL_OFFSET offset) { if (ilArgNum >= info.compArgsCount) { - compInlineResult->setNever("bad argument number"); + compInlineResult->noteFatal(InlineObservation::CALLEE_BAD_ARGUMENT_NUMBER); return; } @@ -13435,7 +13435,7 @@ void Compiler::impLoadLoc(unsigned ilLclNum, IL_OFFSET offset) { if (ilLclNum >= info.compMethodInfo->locals.numArgs) { - compInlineResult->setNever("bad local number"); + compInlineResult->noteFatal(InlineObservation::CALLEE_BAD_LOCAL_NUMBER); return; } @@ -13640,7 +13640,7 @@ bool Compiler::impReturnInstruction(BasicBlock *block, int prefixFlags, OPCODE & if (returnType != originalCallType) { - compInlineResult->setFailure("Return type mismatch"); + compInlineResult->noteFatal(InlineObservation::CALLSITE_RETURN_TYPE_MISMATCH); return false; } @@ -15705,6 +15705,12 @@ void Compiler::impCanInlineNative(int callsiteNativeEst printf("\nmultiplier increased by 10 to %g due to the stress mode.", multiplier); } } + + // Note the various estimates we've obtained. + + inlineResult->noteInt(InlineObservation::CALLEE_NATIVE_SIZE_ESTIMATE, calleeNativeSizeEstimate); + inlineResult->noteInt(InlineObservation::CALLSITE_NATIVE_SIZE_ESTIMATE, callsiteNativeEstimate); + inlineResult->noteDouble(InlineObservation::CALLSITE_BENEFIT_MULTIPLIER, multiplier); #endif @@ -15732,12 +15738,12 @@ void Compiler::impCanInlineNative(int callsiteNativeEst if (pInlineInfo != nullptr) { - inlineResult->setFailure(message); + inlineResult->noteFatal(InlineObservation::CALLSITE_EXCEEDS_THRESHOLD); } else { - // Static hint case.... - inlineResult->setNever(message); + // Static hint case.... here the "callee" is the function being assessed. (PLS VERIFY) + inlineResult->noteFatal(InlineObservation::CALLEE_EXCEEDS_THRESHOLD); } } else @@ -15773,13 +15779,13 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, if (methInfo->EHcount) { - inlineResult->setNever("Inlinee contains EH"); + inlineResult->noteFatal(InlineObservation::CALLEE_HAS_EH); return; } if ((methInfo->ILCode == 0) || (codeSize == 0)) { - inlineResult->setNever("Inlinee has no body"); + inlineResult->noteFatal(InlineObservation::CALLEE_HAS_NO_BODY); return; } @@ -15787,49 +15793,62 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, if (methInfo->args.isVarArg()) { - inlineResult->setNever("Inlinee is vararg"); + inlineResult->noteFatal(InlineObservation::CALLEE_HAS_MANAGED_VARARGS); return; } // 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. - + + inlineResult->noteInt(InlineObservation::CALLEE_NUMBER_OF_LOCALS, methInfo->locals.numArgs); + if (methInfo->locals.numArgs > MAX_INL_LCLS) { - inlineResult->setNever("Inlinee has too many locals"); + inlineResult->noteFatal(InlineObservation::CALLEE_TOO_MANY_LOCALS); return; } // Make sure there aren't too many arguments. // This is currently an implementation limit due to fixed-size arrays in the // inline info, rather than a performance heuristic. + + inlineResult->noteInt(InlineObservation::CALLEE_NUMBER_OF_ARGUMENTS, methInfo->args.numArgs); if (methInfo->args.numArgs > MAX_INL_ARGS) { - inlineResult->setNever("Inlinee has too many arguments"); + inlineResult->noteFatal(InlineObservation::CALLEE_TOO_MANY_ARGUMENTS); return; } if (forceInline) { - inlineResult->setCandidate("Inline is a force inline"); - return; + inlineResult->note(InlineObservation::CALLEE_HAS_FORCE_INLINE); + inlineResult->setCandidate("Inline is a force inline"); + return; } // Reject large functions + inlineResult->noteInt(InlineObservation::CALLEE_NUMBER_OF_IL_BYTES, codeSize); + if (codeSize > impInlineSize) { - inlineResult->setNever("Inlinee is too big"); - return; + inlineResult->note(InlineObservation::CALLEE_TOO_MUCH_IL); + + if (inlineResult->isFailure()) + { + return; + } } // Make sure maxstack is not too big + inlineResult->noteInt(InlineObservation::CALLEE_MAXSTACK, methInfo->maxStack); + if (methInfo->maxStack > sizeof(impSmallStack)/sizeof(impSmallStack[0])) { - inlineResult->setNever("Inlinee's MaxStack is too big"); + inlineResult->noteFatal(InlineObservation::CALLEE_MAXSTACK_TOO_BIG); return; } @@ -15838,41 +15857,38 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, // Check for NetCF quirks mode and the NetCF restrictions if (opts.eeFlags & CORJIT_FLG_NETCF_QUIRKS) { - const char * inlineFailReason = nullptr; - if (codeSize > 16) { - inlineFailReason = "Windows Phone OS 7 compatibility - Method is too big."; - } - else if (methInfo->EHcount) - { - inlineFailReason = "Windows Phone OS 7 compatibility - Inlinee contains EH."; + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_IL_TOO_BIG); + return; } - else if (methInfo->locals.numArgs > 0) + + if (methInfo->EHcount) { - inlineFailReason = "Windows Phone OS 7 compatibility - Inlinee has locals."; + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_HAS_EH); + return; } - else if (methInfo->args.retType == CORINFO_TYPE_FLOAT) + + if (methInfo->locals.numArgs > 0) { - inlineFailReason = "Windows Phone OS 7 compatibility - float return."; + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_HAS_LOCALS); + return; } - else + + if (methInfo->args.retType == CORINFO_TYPE_FLOAT) { - CORINFO_ARG_LIST_HANDLE argLst = methInfo->args.args; - for(unsigned i = methInfo->args.numArgs; i > 0; --i, argLst = info.compCompHnd->getArgNext(argLst)) - { - if (TYP_FLOAT == eeGetArgType(argLst, &methInfo->args)) - { - inlineFailReason = "Windows Phone OS 7 compatibility - float argument."; - break; - } - } + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_HAS_FP_RET); + return; } - if (inlineFailReason != nullptr) + CORINFO_ARG_LIST_HANDLE argLst = methInfo->args.args; + for(unsigned i = methInfo->args.numArgs; i > 0; --i, argLst = info.compCompHnd->getArgNext(argLst)) { - inlineResult->setNever(inlineFailReason); - return; + if (TYP_FLOAT == eeGetArgType(argLst, &methInfo->args)) + { + compInlineResult->noteFatal(InlineObservation::CALLEE_WP7QUIRK_HAS_FP_ARG); + return; + } } } @@ -15931,7 +15947,7 @@ void Compiler::impCheckCanInline(GenTreePtr call, static ConfigDWORD fJitNoInline; if (fJitNoInline.val(CLRConfig::INTERNAL_JitNoInline)) { - pParam->result->setFailure("COMPlus_JitNoInline"); + pParam->result->noteFatal(InlineObservation::CALLEE_IS_JIT_NOINLINE); goto _exit; } #endif @@ -15945,7 +15961,7 @@ void Compiler::impCheckCanInline(GenTreePtr call, CORINFO_METHOD_INFO methInfo; if (!pParam->pThis->info.compCompHnd->getMethodInfo(pParam->fncHandle, &methInfo)) { - pParam->result->setNever("Could not get method info"); + pParam->result->noteFatal(InlineObservation::CALLEE_NO_METHOD_INFO); goto _exit; } @@ -15977,7 +15993,7 @@ void Compiler::impCheckCanInline(GenTreePtr call, if (initClassResult & CORINFO_INITCLASS_DONT_INLINE) { - pParam->result->setFailure("Inlinee's class could not be initialized"); + pParam->result->noteFatal(InlineObservation::CALLSITE_CLASS_INIT_FAILURE_SPEC); goto _exit; } @@ -15994,11 +16010,11 @@ void Compiler::impCheckCanInline(GenTreePtr call, if (vmResult == INLINE_FAIL) { - pParam->result->setFailure("VM rejected inline"); + pParam->result->noteFatal(InlineObservation::CALLSITE_IS_VM_NOINLINE); } else if (vmResult == INLINE_NEVER) { - pParam->result->setNever("VM rejected inline"); + pParam->result->noteFatal(InlineObservation::CALLEE_IS_VM_NOINLINE); } if (pParam->result->isFailure()) @@ -16018,7 +16034,7 @@ void Compiler::impCheckCanInline(GenTreePtr call, if (!pParam->pThis->impIsThis(thisArg)) { - pParam->result->setFailure("Method is called on different this"); + pParam->result->noteFatal(InlineObservation::CALLSITE_REQUIRES_SAME_THIS); goto _exit; } } @@ -16072,7 +16088,7 @@ _exit: } impErrorTrap() { - param.result->setFailure("Exception while inspecting inlining candidate"); + param.result->noteFatal(InlineObservation::CALLSITE_COMPILATION_ERROR); } endErrorTrap(); } @@ -16086,7 +16102,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo * pInlineInfo, if (curArgVal->gtOper == GT_MKREFANY) { - inlineResult->setFailure("Argument contains mkrefany"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_IS_MKREFANY); return; } @@ -16109,7 +16125,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo * pInlineInfo, { // Right now impInlineSpillLclRefs and impInlineSpillGlobEffects don't take // into account special side effects, so we disallow them during inlining. - inlineResult->setFailure("Argument has side effect"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_HAS_SIDE_EFFECT); return; } @@ -16136,7 +16152,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo * pInlineInfo, (curArgVal->gtIntCon.gtIconVal == 0) ) { /* Abort, but do not mark as not inlinable */ - inlineResult->setFailure("Method is called with null this pointer"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_HAS_NULL_THIS); return; } } @@ -16307,7 +16323,7 @@ void Compiler::impInlineInitVars(InlineInfo * pInlineInfo) if (sigType == TYP_REF) { /* The argument cannot be bashed into a ref (see bug 750871) */ - inlineResult->setFailure("Argument is not a ref"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_NO_BASH_TO_REF); return; } @@ -16334,7 +16350,7 @@ void Compiler::impInlineInitVars(InlineInfo * pInlineInfo) else { /* Arguments 'int <- byref' cannot be bashed */ - inlineResult->setFailure("Arguments int <- byref"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_NO_BASH_TO_INT); return; } } @@ -16390,7 +16406,7 @@ void Compiler::impInlineInitVars(InlineInfo * pInlineInfo) if (!isPlausibleTypeMatch) { - inlineResult->setFailure("Arguments incompatible"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_TYPES_INCOMPATIBLE); return; } @@ -16417,7 +16433,7 @@ void Compiler::impInlineInitVars(InlineInfo * pInlineInfo) else { /* Arguments 'int <- byref' cannot be changed */ - inlineResult->setFailure("Arguments int <- byref"); + inlineResult->noteFatal(InlineObservation::CALLSITE_ARG_NO_BASH_TO_INT); return; } } @@ -16491,7 +16507,7 @@ void Compiler::impInlineInitVars(InlineInfo * pInlineInfo) if (isPinned) { - inlineResult->setNever("Method has pinned locals"); + inlineResult->noteFatal(InlineObservation::CALLEE_HAS_PINNED_LOCALS); return; } @@ -16827,7 +16843,7 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT /* Don't inline if not optimized code */ if (opts.compDbgCode) { - inlineResult.setFailure("Compiling debug code"); + inlineResult.noteFatal(InlineObservation::CALLER_DEBUG_CODEGEN); return; } @@ -16835,7 +16851,7 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT // Inlining takes precedence over implicit tail call optimization (if the call is not directly recursive). if (call->IsTailPrefixedCall()) { - inlineResult.setFailure("Call site marked as tailcall"); + inlineResult.noteFatal(InlineObservation::CALLSITE_EXPLICIT_TAIL_PREFIX); return; } @@ -16845,13 +16861,13 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT // as a fast tail call or turned into a loop. if (gtIsRecursiveCall(call) && call->IsImplicitTailCall()) { - inlineResult.setFailure("Recursive tail call"); + inlineResult.noteFatal(InlineObservation::CALLSITE_IMPLICIT_REC_TAIL_CALL); return; } if ((call->gtFlags & GTF_CALL_VIRT_KIND_MASK) != GTF_CALL_NONVIRT) { - inlineResult.setFailure("Not a direct call"); + inlineResult.noteFatal(InlineObservation::CALLSITE_IS_NOT_DIRECT); return; } @@ -16859,14 +16875,14 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT if (call->gtCallType == CT_HELPER) { - inlineResult.setFailure("Inlinee is a helper call"); + inlineResult.noteFatal(InlineObservation::CALLSITE_IS_CALL_TO_HELPER); return; } /* Ignore indirect calls */ if (call->gtCallType == CT_INDIRECT) { - inlineResult.setFailure("Not a direct managed call"); + inlineResult.noteFatal(InlineObservation::CALLSITE_IS_NOT_DIRECT_MANAGED); return; } @@ -16906,7 +16922,7 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT #endif - inlineResult.setFailure("Call site is in a catch handler"); + inlineResult.noteFatal(InlineObservation::CALLSITE_IS_WITHIN_CATCH); return; } @@ -16919,7 +16935,7 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT } #endif - inlineResult.setFailure("Call site is in a filter region"); + inlineResult.noteFatal(InlineObservation::CALLSITE_IS_WITHIN_FILTER); return; } } @@ -16928,7 +16944,7 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT if (opts.compNeedSecurityCheck) { - inlineResult.setFailure("Caller requires a security check"); + inlineResult.noteFatal(InlineObservation::CALLER_NEEDS_SECURITY_CHECK); return; } @@ -16936,15 +16952,21 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT if (methAttr & CORINFO_FLG_DONT_INLINE) { - inlineResult.setFailure("Callee is marked as no inline or has a cached result"); + inlineResult.noteFatal(InlineObservation::CALLEE_IS_NOINLINE); return; } /* Cannot inline native or synchronized methods */ - if (methAttr & (CORINFO_FLG_NATIVE | CORINFO_FLG_SYNCH)) + if (methAttr & CORINFO_FLG_NATIVE) + { + inlineResult.noteFatal(InlineObservation::CALLEE_IS_NATIVE); + return; + } + + if (methAttr & CORINFO_FLG_SYNCH) { - inlineResult.setFailure("Callee is native or synchronized"); + inlineResult.noteFatal(InlineObservation::CALLEE_IS_SYNCHRONIZED); return; } @@ -16952,7 +16974,7 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONT if (methAttr & CORINFO_FLG_SECURITYCHECK) { - inlineResult.setFailure("Callee needs a security check"); + inlineResult.noteFatal(InlineObservation::CALLEE_NEEDS_SECURITY_CHECK); return; } diff --git a/src/coreclr/src/jit/inline.cpp b/src/coreclr/src/jit/inline.cpp new file mode 100644 index 0000000..bb9cd91 --- /dev/null +++ b/src/coreclr/src/jit/inline.cpp @@ -0,0 +1,154 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#include "jitpch.h" +#ifdef _MSC_VER +#pragma hdrstop +#endif + +// Lookup table for inline description strings + +static const char* InlineDescriptions[] = +{ +#define INLINE_OBSERVATION(name, type, description, impact, target) description, +#include "inline.def" +#undef INLINE_OBSERVATION +}; + +// Lookup table for inline targets + +static const InlineTarget InlineTargets[] = +{ +#define INLINE_OBSERVATION(name, type, description, impact, target) InlineTarget::target, +#include "inline.def" +#undef INLINE_OBSERVATION +}; + +// Lookup table for inline impacts + +static const InlineImpact InlineImpacts[] = +{ +#define INLINE_OBSERVATION(name, type, description, impact, target) InlineImpact::impact, +#include "inline.def" +#undef INLINE_OBSERVATION +}; + +#ifdef DEBUG + +//------------------------------------------------------------------------ +// inlIsValidObservation: run a validity check on an inline observation +// +// Arguments: +// obs - the observation in question +// +// Return Value: +// true if the observation is valid + +static bool inlIsValidObservation(InlineObservation obs) +{ + return((obs > InlineObservation::CALLEE_UNUSED_INITIAL) && + (obs < InlineObservation::CALLEE_UNUSED_FINAL)); +} + +#endif // DEBUG + +//------------------------------------------------------------------------ +// inlGetDescriptionString: get a string describing this inline observation +// +// Arguments: +// obs - the observation in question +// +// Return Value: +// string describing the observation + +const char* inlGetDescriptionString(InlineObservation obs) +{ + assert(inlIsValidObservation(obs)); + return InlineDescriptions[static_cast(obs)]; +} + +//------------------------------------------------------------------------ +// inlGetTarget: get the target of an inline observation +// +// Arguments: +// obs - the observation in question +// +// Return Value: +// enum describing the target + +InlineTarget inlGetTarget(InlineObservation obs) +{ + assert(inlIsValidObservation(obs)); + return InlineTargets[static_cast(obs)]; +} + +//------------------------------------------------------------------------ +// inlGetTargetString: get a string describing the target of an inline observation +// +// Arguments: +// obs - the observation in question +// +// Return Value: +// string describing the target + +const char* inlGetTargetstring(InlineObservation obs) +{ + InlineTarget t = inlGetTarget(obs); + switch (t) + { + case InlineTarget::CALLER: + return "caller"; + case InlineTarget::CALLEE: + return "callee"; + case InlineTarget::CALLSITE: + return "call site"; + default: + return "unexpected target"; + } +} + +//------------------------------------------------------------------------ +// inlGetImpact: get the impact of an inline observation +// +// Arguments: +// obs - the observation in question +// +// Return Value: +// enum value describing the impact + +InlineImpact inlGetImpact(InlineObservation obs) +{ + assert(inlIsValidObservation(obs)); + return InlineImpacts[static_cast(obs)]; +} + +//------------------------------------------------------------------------ +// inlGetImpactString: get a string describing the impact of an inline observation +// +// Arguments: +// obs - the observation in question +// +// Return Value: +// string describing the impact + +const char* inlGetImpactString(InlineObservation obs) +{ + InlineImpact i = inlGetImpact(obs); + switch (i) + { + case InlineImpact::FATAL: + return "correctness -- fatal"; + case InlineImpact::FUNDAMENTAL: + return "correctness -- fundamental limitation"; + case InlineImpact::LIMITATION: + return "correctness -- jit limitation"; + case InlineImpact::PERFORMANCE: + return "performance"; + case InlineImpact::INFORMATION: + return "information"; + default: + return "unexpected impact"; + } +} + diff --git a/src/coreclr/src/jit/inline.def b/src/coreclr/src/jit/inline.def new file mode 100644 index 0000000..f054eb5 --- /dev/null +++ b/src/coreclr/src/jit/inline.def @@ -0,0 +1,153 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +// Macro template for inline observations +// +// INLINE_OBSERVATION(name, type, description, impact, target) +// +// name will be used to create an InlineObservation enum member +// (enum name prepends scope, eg CALLEE_MARKED_AS_SKIPPED) +// type is the data type for the observation +// description is a user string for diagnostics +// impact is one of the members of InlineImpact +// target is one of the members of InlineTarget +// +// Note: the impact classification is work in progress. +// +// Some subset of the FATAL cases here can be refined to SERIOUS, +// LIMITATION, or PERFORMANCE. While the refined observations may +// eventually veto inlining, the jit can safely keep making more +// observations. + +// ------ Initial Sentinel ------- + +INLINE_OBSERVATION(UNUSED_INITIAL, bool, "unused initial observation", FATAL, CALLEE) + +// ------ Callee Fatal ------- + +INLINE_OBSERVATION(BAD_ARGUMENT_NUMBER, bool, "invalid argument number", FATAL, CALLEE) +INLINE_OBSERVATION(BAD_LOCAL_NUMBER, bool, "invalid local number", FATAL, CALLEE) +INLINE_OBSERVATION(CLASS_INIT_FAILURE, bool, "class init failed", FATAL, CALLEE) +INLINE_OBSERVATION(COMPILATION_ERROR, bool, "compilation error", FATAL, CALLEE) +INLINE_OBSERVATION(EXCEEDS_THRESHOLD, bool, "exceeds profit threshold", FATAL, CALLEE) +INLINE_OBSERVATION(HAS_DELEGATE_INVOKE, bool, "delegate invoke", FATAL, CALLEE) +INLINE_OBSERVATION(HAS_EH, bool, "has exception handling", FATAL, CALLEE) +INLINE_OBSERVATION(HAS_ENDFILTER, bool, "has endfilter", FATAL, CALLEE) +INLINE_OBSERVATION(HAS_ENDFINALLY, bool, "has endfinally", FATAL, CALLEE) +INLINE_OBSERVATION(HAS_LEAVE, bool, "has leave" , FATAL, CALLEE) +INLINE_OBSERVATION(HAS_MANAGED_VARARGS, bool, "managed varargs", FATAL, CALLEE) +INLINE_OBSERVATION(HAS_NATIVE_VARARGS, bool, "native varargs", FATAL, CALLEE) +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) +INLINE_OBSERVATION(IS_NATIVE, bool, "is implemented natively", FATAL, CALLEE) +INLINE_OBSERVATION(IS_NOINLINE, bool, "noinline per IL/cached result", FATAL, CALLEE) +INLINE_OBSERVATION(IS_SYNCHRONIZED, bool, "is synchronized", FATAL, CALLEE) +INLINE_OBSERVATION(IS_VM_NOINLINE, bool, "noinline per VM", FATAL, CALLEE) +INLINE_OBSERVATION(LACKS_RETURN, bool, "no return opcode", FATAL, CALLEE) +INLINE_OBSERVATION(LDFLD_NEEDS_HELPER, bool, "ldfld needs helper", FATAL, CALLEE) +INLINE_OBSERVATION(MARKED_AS_SKIPPED, bool, "skipped by complus request", FATAL, CALLEE) +INLINE_OBSERVATION(MAXSTACK_TOO_BIG, bool, "maxstack too big" , FATAL, CALLEE) +INLINE_OBSERVATION(NEEDS_SECURITY_CHECK, bool, "needs security check", FATAL, CALLEE) +INLINE_OBSERVATION(NO_METHOD_INFO, bool, "cannot get method info", FATAL, CALLEE) +INLINE_OBSERVATION(RETURN_TYPE_IS_COMPOSITE, bool, "has composite return type", FATAL, CALLEE) +INLINE_OBSERVATION(STACK_CRAWL_MARK, bool, "uses stack crawl mark", FATAL, CALLEE) +INLINE_OBSERVATION(STFLD_NEEDS_HELPER, bool, "stfld needs helper", FATAL, CALLEE) +INLINE_OBSERVATION(STORES_TO_ARGUMENT, bool, "stores to argument", FATAL, CALLEE) +INLINE_OBSERVATION(THROW_WITH_INVALID_STACK, bool, "throw with invalid stack", FATAL, CALLEE) +INLINE_OBSERVATION(TOO_MANY_ARGUMENTS, bool, "too many arguments", FATAL, CALLEE) +INLINE_OBSERVATION(TOO_MANY_LOCALS, bool, "too many locals", FATAL, CALLEE) +INLINE_OBSERVATION(UNSUPPORTED_OPCODE, bool, "unsupported opcode", FATAL, CALLEE) + +#ifdef FEATURE_LEGACYNETCF + +INLINE_OBSERVATION(WP7QUIRK_ADDRESS_TAKEN, bool, "WinPhone7 quirk address taken", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_CONTROL_FLOW, bool, "WinPhone7 quirk has ctl flow", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_HAS_EH, bool, "WinPhone7 quirk has eh", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_HAS_FP_ARG, bool, "WinPhone7 quirk has float arg", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_HAS_FP_RET, bool, "WinPhone7 quirk has float ret", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_HAS_LOCALS, bool, "WinPhone7 quirk has locals", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_IL_TOO_BIG, bool, "WinPhone7 quirk il too big", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_LDARG_ORDER, bool, "WinPhone7 quirk ldarg order", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_PREFIX, bool, "WinPhone7 quirk has prefix", FATAL, CALLEE) +INLINE_OBSERVATION(WP7QUIRK_THROW, bool, "WinPhone7 quirk has throw", FATAL, CALLEE) + +#endif // FEATURE_LEGACYNETCF + +// ------ Callee Performance ------- + +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) + +// ------ Callee Information ------- + +INLINE_OBSERVATION(HAS_FORCE_INLINE, bool, "has aggressive inline attr", 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_IL_BYTES, int, "number of bytes of IL", INFORMATION, CALLEE) +INLINE_OBSERVATION(NUMBER_OF_LOCALS, int, "number of locals", INFORMATION, CALLEE) + +// ------ Caller Corectness ------- + +INLINE_OBSERVATION(DEBUG_CODEGEN, bool, "debug codegen", FATAL, CALLER) +INLINE_OBSERVATION(NEEDS_SECURITY_CHECK, bool, "needs security check", FATAL, CALLER) + +// ------ Call Site Correctness ------- + +INLINE_OBSERVATION(ARG_HAS_NULL_THIS, bool, "this pointer argument is null", FATAL, CALLSITE) +INLINE_OBSERVATION(ARG_HAS_SIDE_EFFECT, bool, "argument has side effect", FATAL, CALLSITE) +INLINE_OBSERVATION(ARG_IS_MKREFANY, bool, "argument is mkrefany", FATAL, CALLSITE) +INLINE_OBSERVATION(ARG_NO_BASH_TO_INT, bool, "argument can't bash to int", FATAL, CALLSITE) +INLINE_OBSERVATION(ARG_NO_BASH_TO_REF, bool, "argument can't bash to ref", FATAL, CALLSITE) +INLINE_OBSERVATION(ARG_TYPES_INCOMPATIBLE, bool, "argument types incompatible", FATAL, CALLSITE) +INLINE_OBSERVATION(CANT_EMBED_PINVOKE_COOKIE, bool, "can't embed pinvoke cookie", FATAL, CALLSITE) +INLINE_OBSERVATION(CANT_EMBED_VARARGS_COOKIE, bool, "can't embed varargs cookie", FATAL, CALLSITE) +INLINE_OBSERVATION(CLASS_INIT_FAILURE_SPEC, bool, "speculative class init failed", FATAL, CALLSITE) +INLINE_OBSERVATION(COMPILATION_ERROR, bool, "compilation error", FATAL, CALLSITE) +INLINE_OBSERVATION(COMPILATION_FAILURE, bool, "failed to compile", FATAL, CALLSITE) +INLINE_OBSERVATION(CONDITIONAL_THROW, bool, "conditional throw", FATAL, CALLSITE) +INLINE_OBSERVATION(CROSS_BOUNDARY_CALLI, bool, "cross-boundary calli", FATAL, CALLSITE) +INLINE_OBSERVATION(CROSS_BOUNDARY_SECURITY, bool, "cross-boundary security check", FATAL, CALLSITE) +INLINE_OBSERVATION(EXCEEDS_THRESHOLD, bool, "exeeds profit threshold", FATAL, CALLSITE) +INLINE_OBSERVATION(EXPLICIT_TAIL_PREFIX, bool, "explicit tail prefix", FATAL, CALLSITE) +INLINE_OBSERVATION(GENERIC_DICTIONARY_LOOKUP, bool, "runtime dictionary lookup", FATAL, CALLSITE) +INLINE_OBSERVATION(HAS_CALL_VIA_LDVIRTFTN, bool, "call via ldvirtftn", FATAL, CALLSITE) +INLINE_OBSERVATION(HAS_COMPLEX_HANDLE, bool, "complex handle access", FATAL, CALLSITE) +INLINE_OBSERVATION(HAS_LDSTR_RESTRICTION, bool, "has ldstr VM restriction", FATAL, CALLSITE) +INLINE_OBSERVATION(IMPLICIT_REC_TAIL_CALL, bool, "implicit recursive tail call", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_CALL_TO_HELPER, bool, "target is helper", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_NOT_DIRECT, bool, "target not direct", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_NOT_DIRECT_MANAGED, bool, "target not direct managed", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_RECURSIVE_OR_DEEP, bool, "recursive or too deep", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_VIRTUAL, bool, "virtual", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_VM_NOINLINE, bool, "noinline per VM", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_WITHIN_CATCH, bool, "within catch region", FATAL, CALLSITE) +INLINE_OBSERVATION(IS_WITHIN_FILTER, bool, "within filterregion", FATAL, CALLSITE) +INLINE_OBSERVATION(LDARGA_NOT_LOCAL_VAR, bool, "ldarga not on local var", FATAL, CALLSITE) +INLINE_OBSERVATION(LDFLD_NEEDS_HELPER, bool, "ldfld needs helper", FATAL, CALLSITE) +INLINE_OBSERVATION(LDVIRTFN_ON_NON_VIRTUAL, bool, "ldvirtfn on non-virtual", FATAL, CALLSITE) +INLINE_OBSERVATION(NOT_CANDIDATE, bool, "not inline candidate", FATAL, CALLSITE) +INLINE_OBSERVATION(REQUIRES_SAME_THIS, bool, "requires same this", FATAL, CALLSITE) +INLINE_OBSERVATION(RETURN_TYPE_MISMATCH, bool, "return type mismatch", FATAL, CALLSITE) +INLINE_OBSERVATION(STFLD_NEEDS_HELPER, bool, "stfld needs helper", FATAL, CALLSITE) +INLINE_OBSERVATION(TOO_MANY_LOCALS, bool, "too many locals", FATAL, CALLSITE) + +// ------ Call Site Peformance ------- + + +// ------ Call Site Information ------- + +INLINE_OBSERVATION(BENEFIT_MULTIPLIER, double, "benefit multiplier", INFORMATION, CALLSITE) +INLINE_OBSERVATION(NATIVE_SIZE_ESTIMATE, double, "native size estimate", INFORMATION, CALLSITE) + +// ------ Final Sentinel ------- + +INLINE_OBSERVATION(UNUSED_FINAL, bool, "unused final observation", FATAL, CALLEE) + diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h new file mode 100644 index 0000000..6be3d46 --- /dev/null +++ b/src/coreclr/src/jit/inline.h @@ -0,0 +1,72 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#ifndef _INLINE_H_ +#define _INLINE_H_ + +// InlineDecision describes the various states the jit goes through when +// evaluating an inline candidate. It is distinct from CorInfoInline +// because it must capture internal states that don't get reported back +// to the runtime. + +enum class InlineDecision +{ + UNDECIDED, + CANDIDATE, + SUCCESS, + FAILURE, + NEVER +}; + +// Possible targets of an inline observation + +enum class InlineTarget +{ + CALLEE, // observation applies to all calls to this callee + CALLER, // observation applies to all calls made by this caller + CALLSITE // observation applies to a specific call site +}; + +// Possible impact of an inline observation + +enum class InlineImpact +{ + FATAL, // inlining impossible, unsafe to evaluate further + FUNDAMENTAL, // inlining impossible for fundamental reasons, deeper exploration safe + LIMITATION, // inlining impossible because of jit limitations, deeper exploration safe + PERFORMANCE, // inlining inadvisable because of performance concerns + INFORMATION // policy-free observation to provide data for later decision making +}; + +// The set of possible inline observations + +enum class InlineObservation +{ +#define INLINE_OBSERVATION(name, type, description, impact, scope) scope ## _ ## name, +#include "inline.def" +#undef INLINE_OBSERVATION +}; + +// Get a string describing this observation + +const char* inlGetDescriptionString(InlineObservation obs); + +// Get a string describing the target of this observation + +const char* inlGetTargetString(InlineObservation obs); + +// Get a string describing the impact of this observation + +const char* inlGetImpactString(InlineObservation obs); + +// Get the target of this observation + +InlineTarget inlGetTarget(InlineObservation obs); + +// Get the impact of this observation + +InlineImpact inlGetImpact(InlineObservation obs); + +#endif // _INLINE_H_ + diff --git a/src/coreclr/src/jit/jit.settings.targets b/src/coreclr/src/jit/jit.settings.targets index dae7894..5f93c539 100644 --- a/src/coreclr/src/jit/jit.settings.targets +++ b/src/coreclr/src/jit/jit.settings.targets @@ -80,6 +80,7 @@ + diff --git a/src/coreclr/src/jit/jitpch.h b/src/coreclr/src/jit/jitpch.h index e50fc76..7c333a7 100644 --- a/src/coreclr/src/jit/jitpch.h +++ b/src/coreclr/src/jit/jitpch.h @@ -28,3 +28,5 @@ #include "ssaconfig.h" #include "blockset.h" #include "bitvec.h" +#include "inline.h" + diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 96a8240..e936cd0 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -5659,14 +5659,19 @@ void Compiler::fgMorphCallInlineHelper(GenTreeCall* call, JitInlineResult* resul { if (lvaCount >= MAX_LV_NUM_COUNT_FOR_INLINING) { - result->setFailure("Too many local variables in the inliner"); + // For now, attributing this to call site, though it's really + // more of a budget issue (lvaCount currently includes all + // caller and prospective callee locals). We still might be + // able to inline other callees into this caller, or inline + // this callee in other callers. + result->noteFatal(InlineObservation::CALLSITE_TOO_MANY_LOCALS); return; } if (call->IsVirtual()) { - result->setFailure("Virtual call"); - return; + result->noteFatal(InlineObservation::CALLSITE_IS_VIRTUAL); + return; } // impMarkInlineCandidate() is expected not to mark tail prefixed calls @@ -5681,14 +5686,14 @@ void Compiler::fgMorphCallInlineHelper(GenTreeCall* call, JitInlineResult* resul if (opts.compNeedSecurityCheck) { - result->setFailure("Caller needs security check"); + result->noteFatal(InlineObservation::CALLER_NEEDS_SECURITY_CHECK); return; } if ((call->gtFlags & GTF_CALL_INLINE_CANDIDATE) == 0) { - result->setFailure("Not an inline candidate"); - return; + result->noteFatal(InlineObservation::CALLSITE_NOT_CANDIDATE); + return; } // -- 2.7.4