Skip to content

Commit

Permalink
JIT: fix phase status for some minimal instrumentation cases (dotnet#…
Browse files Browse the repository at this point in the history
…85411)

The instrumentation phase may modify the flow graph in anticipation of doing
some instrumentation, but then (because of minimal profiling) decide not to
instrument.

Make sure the phase status properly reflects that the phase made changes in
this case, and a few other cases where we exit early without actually doing
any instrumentation.

Fixes dotnet#85061.
  • Loading branch information
AndyAyersMS committed Apr 26, 2023
1 parent 2430618 commit 7ffcee5
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2455,6 +2455,12 @@ PhaseStatus Compiler::fgInstrumentMethod()
}
}

// Even though we haven't yet instrumented, we may have made changes in anticipation...
//
const bool madeAnticipatoryChanges = fgCountInstrumentor->ModifiedFlow() || fgHistogramInstrumentor->ModifiedFlow();
const PhaseStatus earlyExitPhaseStatus =
madeAnticipatoryChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;

// Optionally, when jitting, if there were no class probes and only one count probe,
// suppress instrumentation.
//
Expand All @@ -2478,13 +2484,14 @@ PhaseStatus Compiler::fgInstrumentMethod()
{
JITDUMP(
"Not instrumenting method: minimal probing enabled, and method has only one counter and no class probes\n");
return PhaseStatus::MODIFIED_NOTHING;

return earlyExitPhaseStatus;
}

if (schema.size() == 0)
{
JITDUMP("Not instrumenting method: no schemas were created\n");
return PhaseStatus::MODIFIED_NOTHING;
return earlyExitPhaseStatus;
}

JITDUMP("Instrumenting method: %d count probes and %d class probes\n", fgCountInstrumentor->SchemaCount(),
Expand Down Expand Up @@ -2515,13 +2522,9 @@ PhaseStatus Compiler::fgInstrumentMethod()
if (res != E_NOTIMPL)
{
noway_assert(!"Error: unexpected hresult from allocPgoInstrumentationBySchema");
return PhaseStatus::MODIFIED_NOTHING;
}

// We may have modified control flow preparing for instrumentation.
//
const bool modifiedFlow = fgCountInstrumentor->ModifiedFlow() || fgHistogramInstrumentor->ModifiedFlow();
return modifiedFlow ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
return earlyExitPhaseStatus;
}

JITDUMP("Instrumentation data base address is %p\n", dspPtr(profileMemory));
Expand Down

0 comments on commit 7ffcee5

Please sign in to comment.