Skip to content

Commit 34545d7

Browse files
authored
JIT: don't mark callees noinline for non-fatal observations with pgo (#114821)
Some inline policy decisions are sensitive to the root method having valid PGO data. And sometimes the decision is that a particular callee can never be a viable inline. If so, the policy will have the runtime mark the method as `NoInlining` to short-circuit future inlining decisions and save some JIT throughput. But if the method had valid PGO data the policy may have made a different decision. By marking this method noinline, the JIT will immediately reject all future inline attempts (say from other calling methods with valid PGO data). For instance, without PGO, the policy may limit the number of basic blocks in a viable callee to 5. But with PGO the policy is willing to consider callees with many more blocks. In this PR we modify the reporting policy so that if TieredPGO is active, we only ever report back methods with truly fatal inlining issues, not just "informational" or "performance" issues, regardless of the state of the PGO data for the method being jitted.
1 parent a484803 commit 34545d7

File tree

1 file changed

+26
-3
lines changed

1 file changed

+26
-3
lines changed

src/coreclr/jit/inline.cpp

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -792,16 +792,39 @@ void InlineResult::Report()
792792
// IS_NOINLINE, then we've uncovered a reason why this method
793793
// can't ever be inlined. Update the callee method attributes
794794
// so that future inline attempts for this callee fail faster.
795-
795+
//
796796
InlineObservation obs = m_Policy->GetObservation();
797797

798-
if ((m_Callee != nullptr) && (obs != InlineObservation::CALLEE_IS_NOINLINE))
798+
bool report = (m_Callee != nullptr);
799+
bool suppress = (obs == InlineObservation::CALLEE_IS_NOINLINE);
800+
bool dynamicPgo = m_RootCompiler->fgPgoDynamic;
801+
802+
// If dynamic pgo is active, only propagate noinline back to metadata
803+
// when there is a CALLEE FATAL observation. We want to make sure
804+
// not to block future inlines based on performance or throughput considerations.
805+
//
806+
// Note fgPgoDynamic (and hence dynamicPgo) is true iff TieredPGO is enabled globally.
807+
// In particular this value does not depend on the root method having PGO data.
808+
//
809+
if (dynamicPgo)
810+
{
811+
InlineTarget target = InlGetTarget(obs);
812+
InlineImpact impact = InlGetImpact(obs);
813+
suppress = (target != InlineTarget::CALLEE) || (impact != InlineImpact::FATAL);
814+
}
815+
816+
if (report && !suppress)
799817
{
800-
JITDUMP("\nINLINER: Marking %s as NOINLINE because of %s\n", callee, InlGetObservationString(obs));
818+
JITDUMP("\nINLINER: Marking %s as NOINLINE (observation %s)\n", callee, InlGetObservationString(obs));
801819

802820
COMP_HANDLE comp = m_RootCompiler->info.compCompHnd;
803821
comp->setMethodAttribs(m_Callee, CORINFO_FLG_BAD_INLINEE);
804822
}
823+
else if (suppress)
824+
{
825+
JITDUMP("\nINLINER: Not marking %s NOINLINE; %s (observation %s)\n", callee,
826+
dynamicPgo ? "pgo active" : "already known", InlGetObservationString(obs));
827+
}
805828
}
806829

807830
if (IsDecided() || m_reportFailureAsVmFailure || m_successResult != INLINE_PASS)

0 commit comments

Comments
 (0)