From 1a21536c4d802ee5c42d277922a4da11d7bf3e4c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 7 Feb 2023 10:53:15 +0100 Subject: [PATCH 1/3] Clean up --- src/coreclr/jit/fgprofile.cpp | 52 +++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index bc05a27da850cf..7fb4c2a88db549 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -308,6 +308,10 @@ class Instrumentor { return false; } + virtual bool ShouldInstrument(BasicBlock* block) + { + return ShouldProcess(block); + } virtual void Prepare(bool preImport) { } @@ -320,9 +324,6 @@ class Instrumentor virtual void InstrumentMethodEntry(Schema& schema, uint8_t* profileMemory) { } - virtual void SuppressProbes() - { - } unsigned SchemaCount() const { return m_schemaCount; @@ -1220,15 +1221,17 @@ class EfficientEdgeCountInstrumentor : public Instrumentor, public SpanningTreeV unsigned m_probeCount; unsigned m_edgeProbeCount; bool m_badcode; + bool m_minimal; public: - EfficientEdgeCountInstrumentor(Compiler* comp) + EfficientEdgeCountInstrumentor(Compiler* comp, bool minimal) : Instrumentor(comp) , SpanningTreeVisitor() , m_blockCount(0) , m_probeCount(0) , m_edgeProbeCount(0) , m_badcode(false) + , m_minimal(minimal) { } void Prepare(bool isPreImport) override; @@ -1236,6 +1239,10 @@ class EfficientEdgeCountInstrumentor : public Instrumentor, public SpanningTreeV { return ((block->bbFlags & BBF_IMPORTED) == BBF_IMPORTED); } + bool ShouldInstrument(BasicBlock* block) override + { + return ShouldProcess(block) && ((!m_minimal) || (m_schemaCount > 1)); + } void BuildSchemaElements(BasicBlock* block, Schema& schema) override; void Instrument(BasicBlock* block, Schema& schema, uint8_t* profileMemory) override; @@ -2158,17 +2165,25 @@ PhaseStatus Compiler::fgPrepareToInstrumentMethod() // * disabled by option // * we are prejitting // - const bool edgesEnabled = (JitConfig.JitEdgeProfiling() > 0); - const bool prejit = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT); - const bool useEdgeProfiles = edgesEnabled && !prejit; + const bool edgesEnabled = (JitConfig.JitEdgeProfiling() > 0); + const bool prejit = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT); + const bool useEdgeProfiles = edgesEnabled && !prejit; + const bool minimalProfiling = prejit ? (JitConfig.JitMinimalPrejitProfiling() > 0) : (JitConfig.JitMinimalJitProfiling() > 0); - if (useEdgeProfiles) + if (minimalProfiling && (fgBBcount < 2)) + { + // Don't instrumenting small single-block methods. + JITDUMP("Not using any block profiling (fgBBcount < 2)\n"); + fgCountInstrumentor = new (this, CMK_Pgo) NonInstrumentor(this); + } + else if (useEdgeProfiles) { - fgCountInstrumentor = new (this, CMK_Pgo) EfficientEdgeCountInstrumentor(this); + JITDUMP("Using edge profiling\n"); + fgCountInstrumentor = new (this, CMK_Pgo) EfficientEdgeCountInstrumentor(this, minimalProfiling); } else { - JITDUMP("Using block profiling, because %s\n", prejit ? "prejitting" : "edge profiling disabled"); + JITDUMP("Using block profiling\n"); fgCountInstrumentor = new (this, CMK_Pgo) BlockCountInstrumentor(this); } @@ -2265,6 +2280,12 @@ PhaseStatus Compiler::fgInstrumentMethod() return PhaseStatus::MODIFIED_NOTHING; } + if (schema.size() == 0) + { + JITDUMP("Not instrumenting method: no schemas were created\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + JITDUMP("Instrumenting method: %d count probes and %d class probes\n", fgCountInstrumentor->SchemaCount(), fgHistogramInstrumentor->SchemaCount()); @@ -2296,11 +2317,6 @@ PhaseStatus Compiler::fgInstrumentMethod() return PhaseStatus::MODIFIED_NOTHING; } - // Do any cleanup we might need to do... - // - fgCountInstrumentor->SuppressProbes(); - fgHistogramInstrumentor->SuppressProbes(); - // We may have modified control flow preparing for instrumentation. // const bool modifiedFlow = fgCountInstrumentor->ModifiedFlow() || fgHistogramInstrumentor->ModifiedFlow(); @@ -2313,12 +2329,12 @@ PhaseStatus Compiler::fgInstrumentMethod() // for (BasicBlock* const block : Blocks()) { - if (fgCountInstrumentor->ShouldProcess(block)) + if (fgCountInstrumentor->ShouldInstrument(block)) { fgCountInstrumentor->Instrument(block, schema, profileMemory); } - if (fgHistogramInstrumentor->ShouldProcess(block)) + if (fgHistogramInstrumentor->ShouldInstrument(block)) { fgHistogramInstrumentor->Instrument(block, schema, profileMemory); } @@ -2326,7 +2342,7 @@ PhaseStatus Compiler::fgInstrumentMethod() // Verify we instrumented everything we created schemas for. // - assert(fgCountInstrumentor->InstrCount() == fgCountInstrumentor->SchemaCount()); + assert(fgCountInstrumentor->InstrCount() <= fgCountInstrumentor->SchemaCount()); // Verify we instrumented for each probe // From b87c8082207dce8ec1baf112d154659838500ac2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 7 Feb 2023 11:41:00 +0100 Subject: [PATCH 2/3] make formatter happy --- src/coreclr/jit/fgprofile.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 7fb4c2a88db549..ddf38bc9b52cab 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -2165,10 +2165,11 @@ PhaseStatus Compiler::fgPrepareToInstrumentMethod() // * disabled by option // * we are prejitting // - const bool edgesEnabled = (JitConfig.JitEdgeProfiling() > 0); - const bool prejit = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT); - const bool useEdgeProfiles = edgesEnabled && !prejit; - const bool minimalProfiling = prejit ? (JitConfig.JitMinimalPrejitProfiling() > 0) : (JitConfig.JitMinimalJitProfiling() > 0); + const bool edgesEnabled = (JitConfig.JitEdgeProfiling() > 0); + const bool prejit = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT); + const bool useEdgeProfiles = edgesEnabled && !prejit; + const bool minimalProfiling = + prejit ? (JitConfig.JitMinimalPrejitProfiling() > 0) : (JitConfig.JitMinimalJitProfiling() > 0); if (minimalProfiling && (fgBBcount < 2)) { From cf24d8560a4ccb25195a4123b013db1783004f11 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 7 Feb 2023 11:46:20 +0100 Subject: [PATCH 3/3] revert jitdump change --- src/coreclr/jit/fgprofile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index ddf38bc9b52cab..3c44a6c1059418 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -2184,7 +2184,7 @@ PhaseStatus Compiler::fgPrepareToInstrumentMethod() } else { - JITDUMP("Using block profiling\n"); + JITDUMP("Using block profiling, because %s\n", prejit ? "prejitting" : "edge profiling disabled"); fgCountInstrumentor = new (this, CMK_Pgo) BlockCountInstrumentor(this); }