Skip to content

Commit

Permalink
JIT: fix issues with profile incorporation phase (#47723)
Browse files Browse the repository at this point in the history
Change #47646 did not remove the code in `fgMakeBasicBlocks` that set profile
data. That masked some flaws in `fgIncorporateProfileData`:
* it should process all blocks; nothing has been imported yet
* it must honor a constraint that some handler blocks can't be rare.

This fixes the above, and removes the early profile setting.
  • Loading branch information
AndyAyersMS authored Feb 2, 2021
1 parent 8aca691 commit e7f4dcd
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 36 deletions.
29 changes: 0 additions & 29 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2206,35 +2206,6 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
curBBdesc->bbCodeOffs = curBBoffs;
curBBdesc->bbCodeOffsEnd = nxtBBoffs;

BasicBlock::weight_t profileWeight;

if (fgGetProfileWeightForBasicBlock(curBBoffs, &profileWeight))
{
if (compIsForInlining())
{
if (impInlineInfo->profileScaleState == InlineInfo::ProfileScaleState::KNOWN)
{
double scaledWeight = impInlineInfo->profileScaleFactor * profileWeight;
profileWeight = (BasicBlock::weight_t)scaledWeight;
}
}

curBBdesc->setBBProfileWeight(profileWeight);

if (profileWeight == 0)
{
curBBdesc->bbSetRunRarely();
}
else
{
// Note that bbNewBasicBlock (called from fgNewBasicBlock) may have
// already marked the block as rarely run. In that case (and when we know
// that the block profile weight is non-zero) we want to unmark that.

curBBdesc->bbFlags &= ~BBF_RUN_RARELY;
}
}

switch (jmpKind)
{
case BBJ_SWITCH:
Expand Down
23 changes: 16 additions & 7 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -932,25 +932,24 @@ PhaseStatus Compiler::fgIncorporateProfileData()
// Notes:
// Count data for inlinees is scaled (usually down).
//
// Since we are now running before the importer, we do not know which
// blocks will be imported, and we should not see any internal blocks.
//
// Todo:
// Normalize counts.
//
// Take advantage of the (likely) correspondence between block order
// and schema order?
//
// Find some other mechanism for handling cases where handler entry
// blocks must be in the hot section.
//
void Compiler::fgIncorporateBlockCounts()
{
for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
{
BasicBlock::weight_t profileWeight;

// Skip internal and un-imported blocks.
//
if ((block->bbFlags & (BBF_INTERNAL | BBF_IMPORTED)) != BBF_IMPORTED)
{
continue;
}

if (fgGetProfileWeightForBasicBlock(block->bbCodeOffs, &profileWeight))
{
if (compIsForInlining())
Expand All @@ -972,6 +971,16 @@ void Compiler::fgIncorporateBlockCounts()
{
block->bbFlags &= ~BBF_RUN_RARELY;
}

#if HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION
// Handle a special case -- some handler entries can't have zero profile count.
//
if (this->bbIsHandlerBeg(block) && block->isRunRarely())
{
JITDUMP("Suppressing zero count for " FMT_BB " as it is a handler entry\n", block->bbNum);
block->makeBlockHot();
}
#endif
}
}
}
Expand Down

0 comments on commit e7f4dcd

Please sign in to comment.