JIT: let instrumentor decide which blocks to process

src/coreclr/jit/compiler.h

@@ -5561,7 +5561,7 @@ class Compiler
     bool fgHaveProfileData();
     void fgComputeProfileScale();
     bool fgGetProfileWeightForBasicBlock(IL_OFFSET offset, BasicBlock::weight_t* weight);
-    void fgInstrumentMethod();
+    PhaseStatus fgInstrumentMethod();
 
 public:
     // fgIsUsingProfileWeights - returns true if we have real profile data for this method

src/coreclr/jit/fgprofile.cpp

@@ -229,6 +229,10 @@ class Instrumentor
     }
 
 public:
+    virtual bool ShouldProcess(BasicBlock* block)
+    {
+        return false;
+    }
     virtual void Prepare()
     {
     }
@@ -267,14 +271,18 @@ class NonInstrumentor : public Instrumentor
 
 //------------------------------------------------------------------------
 // BlockCountInstrumentor: instrumentor that adds a counter to each
-//   basic block
+//   non-internal imported basic block
 //
 class BlockCountInstrumentor : public Instrumentor
 {
 public:
     BlockCountInstrumentor(Compiler* comp) : Instrumentor(comp)
     {
     }
+    bool ShouldProcess(BasicBlock* block) override
+    {
+        return ((block->bbFlags & (BBF_INTERNAL | BBF_IMPORTED)) == BBF_IMPORTED);
+    }
     void Prepare() override;
     void BuildSchemaElements(BasicBlock* block, Schema& schema) override;
     void Instrument(BasicBlock* block, Schema& schema, BYTE* profileMemory) override;
@@ -302,6 +310,7 @@ void BlockCountInstrumentor::Prepare()
 //
 // Arguments:
 //   block -- block to instrument
+//   schema -- schema that we're building
 //
 void BlockCountInstrumentor::BuildSchemaElements(BasicBlock* block, Schema& schema)
 {
@@ -621,6 +630,10 @@ class ClassProbeInstrumentor : public Instrumentor
     ClassProbeInstrumentor(Compiler* comp) : Instrumentor(comp)
     {
     }
+    bool ShouldProcess(BasicBlock* block) override
+    {
+        return ((block->bbFlags & (BBF_INTERNAL | BBF_IMPORTED)) == BBF_IMPORTED);
+    }
     void Prepare() override;
     void BuildSchemaElements(BasicBlock* block, Schema& schema) override;
     void Instrument(BasicBlock* block, Schema& schema, BYTE* profileMemory) override;
@@ -648,6 +661,7 @@ void ClassProbeInstrumentor::Prepare()
 //
 // Arguments:
 //   block -- block to instrument
+//   schema -- schema that we're building
 //
 void ClassProbeInstrumentor::BuildSchemaElements(BasicBlock* block, Schema& schema)
 {
@@ -707,7 +721,7 @@ void ClassProbeInstrumentor::Instrument(BasicBlock* block, Schema& schema, BYTE*
 // ClassProbeInstrumentor::SuppressProbes: clean up if we're not instrumenting
 //
 // Notes:
-//   Currently we're hijacking the gtCallStubAddre of the call node to hold
+//   Currently we're hijacking the gtCallStubAddr of the call node to hold
 //   a pointer to the profile candidate info.
 //
 //   We must undo this, if not instrumenting.
@@ -737,6 +751,9 @@ void ClassProbeInstrumentor::SuppressProbes()
 //------------------------------------------------------------------------
 // fgInstrumentMethod: add instrumentation probes to the method
 //
+// Returns:
+//   appropriate phase status
+//
 // Note:
 //
 //   By default this instruments each non-internal block with
@@ -747,7 +764,7 @@ void ClassProbeInstrumentor::SuppressProbes()
 //   Probe structure is described by a schema array, which is created
 //   here based on flowgraph and IR structure.
 //
-void Compiler::fgInstrumentMethod()
+PhaseStatus Compiler::fgInstrumentMethod()
 {
     noway_assert(!compIsForInlining());
 
@@ -775,20 +792,15 @@ void Compiler::fgInstrumentMethod()
     Schema schema(getAllocator(CMK_Pgo));
     for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->bbNext)
     {
-        // Skip internal and un-imported blocks.
-        //
-        if ((block->bbFlags & BBF_IMPORTED) == 0)
+        if (countInst->ShouldProcess(block))
         {
-            continue;
+            countInst->BuildSchemaElements(block, schema);
         }
 
-        if ((block->bbFlags & BBF_INTERNAL) == BBF_INTERNAL)
+        if (classInst->ShouldProcess(block))
         {
-            continue;
+            classInst->BuildSchemaElements(block, schema);
         }
-
-        countInst->BuildSchemaElements(block, schema);
-        classInst->BuildSchemaElements(block, schema);
     }
 
     // Verify we created schema for the calls needing class probes.
@@ -802,7 +814,7 @@ void Compiler::fgInstrumentMethod()
     if ((JitConfig.JitMinimalProfiling() > 0) && (countInst->SchemaCount() == 1) && (classInst->SchemaCount() == 0))
     {
         JITDUMP("Not instrumenting method: only one counter, and no class probes\n");
-        return;
+        return PhaseStatus::MODIFIED_NOTHING;
     }
 
     JITDUMP("Instrumenting method: %d count probes and %d class probes\n", countInst->SchemaCount(),
@@ -826,34 +838,29 @@ void Compiler::fgInstrumentMethod()
         if (res != E_NOTIMPL)
         {
             noway_assert(!"Error: unexpected hresult from allocPgoInstrumentationBySchema");
-            return;
+            return PhaseStatus::MODIFIED_NOTHING;
         }
 
         // Do any cleanup we might need to do...
         //
         countInst->SuppressProbes();
         classInst->SuppressProbes();
-        return;
+        return PhaseStatus::MODIFIED_NOTHING;
     }
 
     // Add the instrumentation code
     //
     for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->bbNext)
     {
-        // Skip internal and un-imported blocks.
-        //
-        if ((block->bbFlags & BBF_IMPORTED) == 0)
+        if (countInst->ShouldProcess(block))
         {
-            continue;
+            countInst->Instrument(block, schema, profileMemory);
         }
 
-        if ((block->bbFlags & BBF_INTERNAL) == BBF_INTERNAL)
+        if (classInst->ShouldProcess(block))
         {
-            continue;
+            classInst->Instrument(block, schema, profileMemory);
         }
-
-        countInst->Instrument(block, schema, profileMemory);
-        classInst->Instrument(block, schema, profileMemory);
     }
 
     // Verify we instrumented everthing we created schemas for.
@@ -866,6 +873,8 @@ void Compiler::fgInstrumentMethod()
     //
     countInst->InstrumentMethodEntry(schema, profileMemory);
     classInst->InstrumentMethodEntry(schema, profileMemory);
+
+    return PhaseStatus::MODIFIED_EVERYTHING;
 }
 
 bool flowList::setEdgeWeightMinChecked(BasicBlock::weight_t newWeight, BasicBlock::weight_t slop, bool* wbUsedSlop)

src/coreclr/jit/phase.cpp

@@ -157,19 +157,13 @@ void Phase::PostPhase(PhaseStatus status)
     // well as the new-style phases that have been updated to return
     // PhaseStatus from their DoPhase methods.
     //
-    static Phases s_allowlist[] = {PHASE_IMPORTATION,
-                                   PHASE_INDXCALL,
-                                   PHASE_MORPH_INLINE,
-                                   PHASE_ALLOCATE_OBJECTS,
-                                   PHASE_EMPTY_TRY,
-                                   PHASE_EMPTY_FINALLY,
-                                   PHASE_MERGE_FINALLY_CHAINS,
-                                   PHASE_CLONE_FINALLY,
-                                   PHASE_MERGE_THROWS,
-                                   PHASE_MORPH_GLOBAL,
-                                   PHASE_BUILD_SSA,
-                                   PHASE_RATIONALIZE,
-                                   PHASE_LOWERING,
+    static Phases s_allowlist[] = {PHASE_IMPORTATION,       PHASE_IBCINSTR,
+                                   PHASE_INDXCALL,          PHASE_MORPH_INLINE,
+                                   PHASE_ALLOCATE_OBJECTS,  PHASE_EMPTY_TRY,
+                                   PHASE_EMPTY_FINALLY,     PHASE_MERGE_FINALLY_CHAINS,
+                                   PHASE_CLONE_FINALLY,     PHASE_MERGE_THROWS,
+                                   PHASE_MORPH_GLOBAL,      PHASE_BUILD_SSA,
+                                   PHASE_RATIONALIZE,       PHASE_LOWERING,
                                    PHASE_STACK_LEVEL_SETTER};
 
     if (madeChanges)