Skip to content

Commit f1be1df

Browse files
authored
Return false in PropagateNeverToRuntime for BBJ_THROW blocks (#118280)
1 parent 0234029 commit f1be1df

File tree

2 files changed

+44
-34
lines changed

2 files changed

+44
-34
lines changed

src/coreclr/jit/inline.cpp

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,7 @@ void InlineResult::Report()
786786
// Was the result NEVER? If so we might want to propagate this to
787787
// the runtime.
788788

789-
if (IsNever() && m_Policy->PropagateNeverToRuntime())
789+
if (IsNever() && m_Policy->PropagateNeverToRuntime() && (m_Callee != nullptr))
790790
{
791791
// If we know the callee, and if the observation that got us
792792
// to this Never inline state is something *other* than
@@ -795,36 +795,16 @@ void InlineResult::Report()
795795
// so that future inline attempts for this callee fail faster.
796796
//
797797
InlineObservation obs = m_Policy->GetObservation();
798-
799-
bool report = (m_Callee != nullptr);
800-
bool suppress = (obs == InlineObservation::CALLEE_IS_NOINLINE);
801-
bool dynamicPgo = m_RootCompiler->fgPgoDynamic;
802-
803-
// If dynamic pgo is active, only propagate noinline back to metadata
804-
// when there is a CALLEE FATAL observation. We want to make sure
805-
// not to block future inlines based on performance or throughput considerations.
806-
//
807-
// Note fgPgoDynamic (and hence dynamicPgo) is true iff TieredPGO is enabled globally.
808-
// In particular this value does not depend on the root method having PGO data.
809-
//
810-
if (dynamicPgo)
811-
{
812-
InlineTarget target = InlGetTarget(obs);
813-
InlineImpact impact = InlGetImpact(obs);
814-
suppress = (target != InlineTarget::CALLEE) || (impact != InlineImpact::FATAL);
815-
}
816-
817-
if (report && !suppress)
798+
if (obs != InlineObservation::CALLEE_IS_NOINLINE)
818799
{
819800
JITDUMP("\nINLINER: Marking %s as NOINLINE (observation %s)\n", callee, InlGetObservationString(obs));
820801

821802
COMP_HANDLE comp = m_RootCompiler->info.compCompHnd;
822803
comp->setMethodAttribs(m_Callee, CORINFO_FLG_BAD_INLINEE);
823804
}
824-
else if (suppress)
805+
else
825806
{
826-
JITDUMP("\nINLINER: Not marking %s NOINLINE; %s (observation %s)\n", callee,
827-
dynamicPgo ? "pgo active" : "already known", InlGetObservationString(obs));
807+
JITDUMP("\nINLINER: Not marking %s NOINLINE because it's already marked as such\n", callee);
828808
}
829809
}
830810

src/coreclr/jit/inlinepolicy.cpp

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,18 +1036,48 @@ void DefaultPolicy::OnDumpXml(FILE* file, unsigned indent) const
10361036

10371037
bool DefaultPolicy::PropagateNeverToRuntime() const
10381038
{
1039-
//
1040-
// Do not propagate the "no return" observation. If we do this then future inlining
1041-
// attempts will fail immediately without marking the call node as "no return".
1042-
// This can have an adverse impact on caller's code quality as it may have to preserve
1043-
// registers across the call.
1044-
// TODO-Throughput: We should persist the "no return" information in the runtime
1045-
// so we don't need to re-analyze the inlinee all the time.
1046-
//
1039+
if (m_Observation == InlineObservation::CALLEE_DOES_NOT_RETURN)
1040+
{
1041+
// Do not propagate the "no return" observation. If we do this then future inlining
1042+
// attempts will fail immediately without marking the call node as "no return".
1043+
// This can have an adverse impact on caller's code quality as it may have to preserve
1044+
// registers across the call.
1045+
// TODO-Throughput: We should persist the "no return" information in the runtime
1046+
// so we don't need to re-analyze the inlinee all the time.
1047+
//
1048+
return false;
1049+
}
1050+
1051+
InlineTarget target = InlGetTarget(GetObservation());
1052+
InlineImpact impact = InlGetImpact(GetObservation());
1053+
1054+
if ((target == InlineTarget::CALLEE) && (impact == InlineImpact::FATAL))
1055+
{
1056+
// This callee will never inline.
1057+
//
1058+
return true;
1059+
}
10471060

1048-
bool propagate = (m_Observation != InlineObservation::CALLEE_DOES_NOT_RETURN);
1061+
if (m_InsideThrowBlock)
1062+
{
1063+
// We inline only trivial methods inside BBJ_THROW call-sites - no need to record that.
1064+
//
1065+
return false;
1066+
}
1067+
1068+
if (m_RootCompiler->fgPgoDynamic)
1069+
{
1070+
// If dynamic pgo is active, only propagate noinline back to metadata
1071+
// when there is a CALLEE FATAL observation. We want to make sure
1072+
// not to block future inlines based on performance or throughput considerations.
1073+
//
1074+
// Note fgPgoDynamic (and hence dynamicPgo) is true iff TieredPGO is enabled globally.
1075+
// In particular this value does not depend on the root method having PGO data.
1076+
//
1077+
return false;
1078+
}
10491079

1050-
return propagate;
1080+
return true;
10511081
}
10521082

10531083
#if defined(DEBUG)

0 commit comments

Comments
 (0)