From c442ea14a7c5001c0de9b71b9806ff8c210ccb92 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 12 Oct 2018 13:04:06 -0700 Subject: [PATCH 1/2] JIT: add some devirtualization info to the inline context Allows the jit to remember which calls were devirtualized and which of those were then optimized to use an unboxed entry point. This info is then dumped out as part of the inline tree. Also remove some of the clutter from the COMPlus_JitPrintInlinedMethods output stream -- we don't need to see both the in-stream results and the final results, and we don't really need to know about the budget. This information is still dumped for COMPlus_JitDump. --- src/jit/flowgraph.cpp | 7 +++-- src/jit/gentree.h | 12 ++++++++ src/jit/importer.cpp | 5 +++- src/jit/inline.cpp | 68 +++++++++++++++++++++++++++---------------- src/jit/inline.h | 14 ++++++++- 5 files changed, 76 insertions(+), 30 deletions(-) diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index f8fe15da74a3..003ed3c7f8ca 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -21878,8 +21878,9 @@ void Compiler::fgInline() if (verbose || fgPrintInlinedMethods) { - printf("**************** Inline Tree\n"); - m_inlineStrategy->Dump(); + JITDUMP("**************** Inline Tree"); + printf("\n"); + m_inlineStrategy->Dump(verbose); } #endif // DEBUG @@ -22573,7 +22574,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe #ifdef DEBUG - if (verbose || fgPrintInlinedMethods) + if (verbose) { printf("Successfully inlined %s (%d IL bytes) (depth %d) [%s]\n", eeGetMethodFullName(fncHandle), inlineCandidateInfo->methInfo.ILCodeSize, inlineDepth, inlineResult->ReasonString()); diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 3fecf008a694..e75fb380be73 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -3537,6 +3537,8 @@ struct GenTreeCall final : public GenTree // stubs, because executable code cannot be generated at runtime. #define GTF_CALL_M_HELPER_SPECIAL_DCE 0x00020000 // GT_CALL -- this helper call can be removed if it is part of a comma and // the comma result is unused. +#define GTF_CALL_M_DEVIRTUALIZED 0x00040000 // GT_CALL -- this call was devirtualized +#define GTF_CALL_M_UNBOXED 0x00080000 // GT_CALL -- this call was optimized to use the unboxed entry point // clang-format on @@ -3743,6 +3745,16 @@ struct GenTreeCall final : public GenTree gtCallMoreFlags |= GTF_CALL_M_FAT_POINTER_CHECK; } + bool IsDevirtualized() const + { + return (gtCallMoreFlags & GTF_CALL_M_DEVIRTUALIZED) != 0; + } + + bool IsUnboxed() const + { + return (gtCallMoreFlags & GTF_CALL_M_UNBOXED) != 0; + } + unsigned gtCallMoreFlags; // in addition to gtFlags unsigned char gtCallType : 3; // value from the gtCallTypes enumeration diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index a3a82e8ed5df..68f602b71573 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -19631,6 +19631,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, call->gtFlags &= ~GTF_CALL_VIRT_STUB; call->gtCallMethHnd = derivedMethod; call->gtCallType = CT_USER_FUNC; + call->gtCallMoreFlags |= GTF_CALL_M_DEVIRTUALIZED; // Virtual calls include an implicit null check, which we may // now need to make explicit. @@ -19695,6 +19696,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // Pass the local var as this and the type handle as a new arg JITDUMP("Success! invoking unboxed entry point on local copy, and passing method table arg\n"); call->gtCallObjp = localCopyThis; + call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; // Prepend for R2L arg passing or empty L2R passing if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr)) @@ -19742,7 +19744,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, JITDUMP("Success! invoking unboxed entry point on local copy\n"); call->gtCallObjp = localCopyThis; call->gtCallMethHnd = unboxedEntryMethod; - derivedMethod = unboxedEntryMethod; + call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; + derivedMethod = unboxedEntryMethod; } else { diff --git a/src/jit/inline.cpp b/src/jit/inline.cpp index 36a8802f9d18..36046855fdf3 100644 --- a/src/jit/inline.cpp +++ b/src/jit/inline.cpp @@ -336,6 +336,8 @@ InlineContext::InlineContext(InlineStrategy* strategy) , m_Observation(InlineObservation::CALLEE_UNUSED_INITIAL) , m_CodeSizeEstimate(0) , m_Success(true) + , m_Devirtualized(false) + , m_Unboxed(false) #if defined(DEBUG) || defined(INLINE_DATA) , m_Policy(nullptr) , m_Callee(nullptr) @@ -392,19 +394,21 @@ void InlineContext::Dump(unsigned indent) else { // Inline attempt. - const char* inlineReason = InlGetObservationString(m_Observation); - const char* inlineResult = m_Success ? "" : "FAILED: "; + const char* inlineReason = InlGetObservationString(m_Observation); + const char* inlineResult = m_Success ? "" : "FAILED: "; + const char* devirtualized = m_Devirtualized ? " devirt" : ""; + const char* unboxed = m_Unboxed ? " unboxed" : ""; if (m_Offset == BAD_IL_OFFSET) { - printf("%*s[%u IL=???? TR=%06u %08X] [%s%s] %s\n", indent, "", m_Ordinal, m_TreeID, calleeToken, - inlineResult, inlineReason, calleeName); + printf("%*s[%u IL=???? TR=%06u %08X] [%s%s%s%s] %s\n", indent, "", m_Ordinal, m_TreeID, calleeToken, + inlineResult, inlineReason, devirtualized, unboxed, calleeName); } else { IL_OFFSET offset = jitGetILoffs(m_Offset); - printf("%*s[%u IL=%04d TR=%06u %08X] [%s%s] %s\n", indent, "", m_Ordinal, offset, m_TreeID, calleeToken, - inlineResult, inlineReason, calleeName); + printf("%*s[%u IL=%04d TR=%06u %08X] [%s%s%s%s] %s\n", indent, "", m_Ordinal, offset, m_TreeID, calleeToken, + inlineResult, inlineReason, devirtualized, unboxed, calleeName); } } @@ -1186,6 +1190,7 @@ InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo) BYTE* calleeIL = inlineInfo->inlineCandidateInfo->methInfo.ILCode; unsigned calleeILSize = inlineInfo->inlineCandidateInfo->methInfo.ILCodeSize; InlineContext* parentContext = stmt->gtInlineContext; + GenTreeCall* originalCall = inlineInfo->inlineResult->GetCall(); noway_assert(parentContext != nullptr); @@ -1194,12 +1199,14 @@ InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo) calleeContext->m_Parent = parentContext; // Push on front here will put siblings in reverse lexical // order which we undo in the dumper - calleeContext->m_Sibling = parentContext->m_Child; - parentContext->m_Child = calleeContext; - calleeContext->m_Child = nullptr; - calleeContext->m_Offset = stmt->AsStmt()->gtStmtILoffsx; - calleeContext->m_Observation = inlineInfo->inlineResult->GetObservation(); - calleeContext->m_Success = true; + calleeContext->m_Sibling = parentContext->m_Child; + parentContext->m_Child = calleeContext; + calleeContext->m_Child = nullptr; + calleeContext->m_Offset = stmt->AsStmt()->gtStmtILoffsx; + calleeContext->m_Observation = inlineInfo->inlineResult->GetObservation(); + calleeContext->m_Success = true; + calleeContext->m_Devirtualized = originalCall->IsDevirtualized(); + calleeContext->m_Unboxed = originalCall->IsUnboxed(); #if defined(DEBUG) || defined(INLINE_DATA) @@ -1211,13 +1218,13 @@ InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo) // +1 here since we set this before calling NoteOutcome. calleeContext->m_Ordinal = m_InlineCount + 1; // Update offset with more accurate info - calleeContext->m_Offset = inlineInfo->inlineResult->GetCall()->gtRawILOffset; + calleeContext->m_Offset = originalCall->gtRawILOffset; #endif // defined(DEBUG) || defined(INLINE_DATA) #if defined(DEBUG) - calleeContext->m_TreeID = inlineInfo->inlineResult->GetCall()->gtTreeID; + calleeContext->m_TreeID = originalCall->gtTreeID; #endif // defined(DEBUG) @@ -1247,31 +1254,34 @@ InlineContext* InlineStrategy::NewFailure(GenTreeStmt* stmt, InlineResult* inlin InlineContext* parentContext = stmt->gtInlineContext; assert(parentContext != nullptr); InlineContext* failedContext = new (m_Compiler, CMK_Inlining) InlineContext(this); + GenTreeCall* originalCall = inlineResult->GetCall(); // Pushing the new context on the front of the parent child list // will put siblings in reverse lexical order which we undo in the // dumper. - failedContext->m_Parent = parentContext; - failedContext->m_Sibling = parentContext->m_Child; - parentContext->m_Child = failedContext; - failedContext->m_Child = nullptr; - failedContext->m_Offset = stmt->gtStmtILoffsx; - failedContext->m_Observation = inlineResult->GetObservation(); - failedContext->m_Callee = inlineResult->GetCallee(); - failedContext->m_Success = false; + failedContext->m_Parent = parentContext; + failedContext->m_Sibling = parentContext->m_Child; + parentContext->m_Child = failedContext; + failedContext->m_Child = nullptr; + failedContext->m_Offset = stmt->gtStmtILoffsx; + failedContext->m_Observation = inlineResult->GetObservation(); + failedContext->m_Callee = inlineResult->GetCallee(); + failedContext->m_Success = false; + failedContext->m_Devirtualized = originalCall->IsDevirtualized(); + failedContext->m_Unboxed = originalCall->IsUnboxed(); assert(InlIsValidObservation(failedContext->m_Observation)); #if defined(DEBUG) || defined(INLINE_DATA) // Update offset with more accurate info - failedContext->m_Offset = inlineResult->GetCall()->gtRawILOffset; + failedContext->m_Offset = originalCall->gtRawILOffset; #endif // #if defined(DEBUG) || defined(INLINE_DATA) #if defined(DEBUG) - failedContext->m_TreeID = inlineResult->GetCall()->gtTreeID; + failedContext->m_TreeID = originalCall->gtTreeID; #endif // defined(DEBUG) @@ -1282,11 +1292,19 @@ InlineContext* InlineStrategy::NewFailure(GenTreeStmt* stmt, InlineResult* inlin //------------------------------------------------------------------------ // Dump: dump description of inline behavior +// +// Arguments: +// showBudget - also dump final budget values -void InlineStrategy::Dump() +void InlineStrategy::Dump(bool showBudget) { m_RootContext->Dump(); + if (!showBudget) + { + return; + } + printf("Budget: initialTime=%d, finalTime=%d, initialBudget=%d, currentBudget=%d\n", m_InitialTimeEstimate, m_CurrentTimeEstimate, m_InitialTimeBudget, m_CurrentTimeBudget); diff --git a/src/jit/inline.h b/src/jit/inline.h index cedf36d0cebb..0c5d650988d0 100644 --- a/src/jit/inline.h +++ b/src/jit/inline.h @@ -677,6 +677,16 @@ class InlineContext return m_Parent == nullptr; } + bool IsDevirtualized() const + { + return m_Devirtualized; + } + + bool IsUnboxed() const + { + return m_Unboxed; + } + private: InlineContext(InlineStrategy* strategy); @@ -691,6 +701,8 @@ class InlineContext InlineObservation m_Observation; // what lead to this inline int m_CodeSizeEstimate; // in bytes * 10 bool m_Success; // true if this was a successful inline + bool m_Devirtualized; // true if this was a devirtualized call + bool m_Unboxed; // true if this call now invokes the unboxed entry #if defined(DEBUG) || defined(INLINE_DATA) @@ -817,7 +829,7 @@ class InlineStrategy #if defined(DEBUG) || defined(INLINE_DATA) // Dump textual description of inlines done so far. - void Dump(); + void Dump(bool showBudget); // Dump data-format description of inlines done so far. void DumpData(); From 8f381fbadbf592bbb8665ce8215fbd4cc5402dc4 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 12 Oct 2018 15:14:08 -0700 Subject: [PATCH 2/2] review feedback --- src/jit/inline.cpp | 6 ++---- src/jit/inline.h | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/jit/inline.cpp b/src/jit/inline.cpp index 36046855fdf3..a1064762ce7a 100644 --- a/src/jit/inline.cpp +++ b/src/jit/inline.cpp @@ -1176,7 +1176,6 @@ InlineContext* InlineStrategy::NewRoot() // and link it into the context tree // // Arguments: -// stmt - statement containing call being inlined // inlineInfo - information about this inline // // Return Value: @@ -1202,7 +1201,7 @@ InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo) calleeContext->m_Sibling = parentContext->m_Child; parentContext->m_Child = calleeContext; calleeContext->m_Child = nullptr; - calleeContext->m_Offset = stmt->AsStmt()->gtStmtILoffsx; + calleeContext->m_Offset = stmt->gtStmtILoffsx; calleeContext->m_Observation = inlineInfo->inlineResult->GetObservation(); calleeContext->m_Success = true; calleeContext->m_Devirtualized = originalCall->IsDevirtualized(); @@ -1244,8 +1243,7 @@ InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo) // inlineResult - inlineResult for the attempt // // Return Value: -// A new InlineContext for diagnostic purposes, or nullptr if -// the desired context could not be created. +// A new InlineContext for diagnostic purposes InlineContext* InlineStrategy::NewFailure(GenTreeStmt* stmt, InlineResult* inlineResult) { diff --git a/src/jit/inline.h b/src/jit/inline.h index 0c5d650988d0..0503b074ddea 100644 --- a/src/jit/inline.h +++ b/src/jit/inline.h @@ -691,18 +691,18 @@ class InlineContext InlineContext(InlineStrategy* strategy); private: - InlineStrategy* m_InlineStrategy; // overall strategy - InlineContext* m_Parent; // logical caller (parent) - InlineContext* m_Child; // first child - InlineContext* m_Sibling; // next child of the parent - BYTE* m_Code; // address of IL buffer for the method - unsigned m_ILSize; // size of IL buffer for the method - IL_OFFSETX m_Offset; // call site location within parent - InlineObservation m_Observation; // what lead to this inline - int m_CodeSizeEstimate; // in bytes * 10 - bool m_Success; // true if this was a successful inline - bool m_Devirtualized; // true if this was a devirtualized call - bool m_Unboxed; // true if this call now invokes the unboxed entry + InlineStrategy* m_InlineStrategy; // overall strategy + InlineContext* m_Parent; // logical caller (parent) + InlineContext* m_Child; // first child + InlineContext* m_Sibling; // next child of the parent + BYTE* m_Code; // address of IL buffer for the method + unsigned m_ILSize; // size of IL buffer for the method + IL_OFFSETX m_Offset; // call site location within parent + InlineObservation m_Observation; // what lead to this inline + int m_CodeSizeEstimate; // in bytes * 10 + bool m_Success : 1; // true if this was a successful inline + bool m_Devirtualized : 1; // true if this was a devirtualized call + bool m_Unboxed : 1; // true if this call now invokes the unboxed entry #if defined(DEBUG) || defined(INLINE_DATA)