Skip to content

Commit 32414ec

Browse files
authored
JIT: prepare for instrumentation before incorporating profile counts (#85805)
Otherwise the spanning tree we generate may be biased by the profile data and not match the spanning tree we generated in Tier0. Fixes #85799.
1 parent c31fda1 commit 32414ec

File tree

3 files changed

+23
-15
lines changed

3 files changed

+23
-15
lines changed

src/coreclr/jit/block.h

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,15 +1089,13 @@ struct BasicBlock : private LIR::Range
10891089
BlockSet bbReach; // Set of all blocks that can reach this one
10901090

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

1097-
union {
1098-
void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts
1099-
void* bbSparseProbeList; // Used early on by fgInstrument
1100-
};
1098+
void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts
11011099

11021100
unsigned bbPreorderNum; // the block's preorder number in the graph (1...fgMaxBBNum]
11031101
unsigned bbPostorderNum; // the block's postorder number in the graph (1...fgMaxBBNum]

src/coreclr/jit/compiler.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4496,6 +4496,14 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
44964496
};
44974497
DoPhase(this, PHASE_PRE_IMPORT, preImportPhase);
44984498

4499+
// If we're going to instrument code, we may need to prepare before
4500+
// we import. Also do this before we read in any profile data.
4501+
//
4502+
if (compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR))
4503+
{
4504+
DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
4505+
}
4506+
44994507
// Incorporate profile data.
45004508
//
45014509
// Note: the importer is sensitive to block weights, so this has
@@ -4505,14 +4513,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
45054513
DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData);
45064514
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;
45074515

4508-
// If we're going to instrument code, we may need to prepare before
4509-
// we import.
4510-
//
4511-
if (compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR))
4512-
{
4513-
DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
4514-
}
4515-
45164516
// If we are doing OSR, update flow to initially reach the appropriate IL offset.
45174517
//
45184518
if (opts.IsOSR())

src/coreclr/jit/fgprofile.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,6 +1220,16 @@ void Compiler::WalkSpanningTree(SpanningTreeVisitor* visitor)
12201220
break;
12211221
}
12221222
}
1223+
1224+
// Notify visitor of remaining blocks
1225+
//
1226+
for (BasicBlock* const block : Blocks())
1227+
{
1228+
if (!BlockSetOps::IsMember(this, marked, block->bbNum))
1229+
{
1230+
visitor->VisitBlock(block);
1231+
}
1232+
}
12231233
}
12241234

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

0 commit comments

Comments
 (0)