Skip to content

Commit

Permalink
JIT: prepare for instrumentation before incorporating profile counts
Browse files Browse the repository at this point in the history
Otherwise the spanning tree we generate may be biased by the profile data
and not match the spanning tree we generated in Tier0.

Fixes dotnet#85799.
  • Loading branch information
AndyAyersMS committed May 5, 2023
1 parent 4679e59 commit e362533
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
12 changes: 5 additions & 7 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -1089,15 +1089,13 @@ struct BasicBlock : private LIR::Range
BlockSet bbReach; // Set of all blocks that can reach this one

union {
BasicBlock* bbIDom; // Represent the closest dominator to this block (called the Immediate
// Dominator) used to compute the dominance tree.
FlowEdge* bbLastPred; // Used early on by fgLinkBasicBlock/fgAddRefPred
BasicBlock* bbIDom; // Represent the closest dominator to this block (called the Immediate
// Dominator) used to compute the dominance tree.
FlowEdge* bbLastPred; // Used early on by fgLinkBasicBlock/fgAddRefPred
void* bbSparseProbeList; // Used early on by fgInstrument
};

union {
void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts
void* bbSparseProbeList; // Used early on by fgInstrument
};
void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts

unsigned bbPreorderNum; // the block's preorder number in the graph (1...fgMaxBBNum]
unsigned bbPostorderNum; // the block's postorder number in the graph (1...fgMaxBBNum]
Expand Down
16 changes: 8 additions & 8 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4496,6 +4496,14 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
};
DoPhase(this, PHASE_PRE_IMPORT, preImportPhase);

// If we're going to instrument code, we may need to prepare before
// we import. Also do this before we read in any profile data.
//
if (compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR))
{
DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
}

// Incorporate profile data.
//
// Note: the importer is sensitive to block weights, so this has
Expand All @@ -4505,14 +4513,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData);
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;

// If we're going to instrument code, we may need to prepare before
// we import.
//
if (compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR))
{
DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
}

// If we are doing OSR, update flow to initially reach the appropriate IL offset.
//
if (opts.IsOSR())
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,16 @@ void Compiler::WalkSpanningTree(SpanningTreeVisitor* visitor)
break;
}
}

// Notify visitor of remaining blocks
//
for (BasicBlock* const block : Blocks())
{
if (!BlockSetOps::IsMember(this, marked, block->bbNum))
{
visitor->VisitBlock(block);
}
}
}

// Map a block into its schema key we will use for storing basic blocks.
Expand Down

0 comments on commit e362533

Please sign in to comment.