Skip to content

Commit

Permalink
JIT: revise inlinee block numbering, enable synthesis for inlinees
Browse files Browse the repository at this point in the history 
Start numbering inlinee blocks from 1 instead of 1 + the root compiler's
max BB num. Update inlinee block bbNums when they are inserted into the
root compiler's graph.

Adjust computations in various places that knew about the old approach
and looked from inlinee compiler to root compiler for bitset, epochs and
the like.

Enable synthesis for inlinees, now that regular bitsets on inlinee compiler
instances behave sensibly.

There is still some messiness around inlinees inheriting root compiler
EH info which requires special checks. I will clean this up separately.

Fixes dotnet#82755.
Contributes to dotnet#82964.

enable synthesis for inlinees
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Mar 17, 2023
1 parent bbc5634 commit bd63baa
Show file tree
Hide file tree
Showing 10 changed files with 136 additions and 155 deletions.
13 changes: 2 additions & 11 deletions src/coreclr/jit/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ void BasicBlock::dspSuccs(Compiler* compiler)
{
// Create a set with all the successors. Don't use BlockSet, so we don't need to worry
// about the BlockSet epoch.
unsigned bbNumMax = compiler->impInlineRoot()->fgBBNumMax;
unsigned bbNumMax = compiler->fgBBNumMax;
BitVecTraits bitVecTraits(bbNumMax + 1, compiler);
BitVec uniqueSuccBlocks(BitVecOps::MakeEmpty(&bitVecTraits));
for (BasicBlock* const bTarget : SwitchTargets())
Expand Down Expand Up @@ -1431,16 +1431,7 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)
/* Give the block a number, set the ancestor count and weight */

++fgBBcount;

if (compIsForInlining())
{
block->bbNum = ++impInlineInfo->InlinerCompiler->fgBBNumMax;
fgBBNumMax = block->bbNum;
}
else
{
block->bbNum = ++fgBBNumMax;
}
block->bbNum = ++fgBBNumMax;

if (compRationalIRForm)
{
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4402,7 +4402,6 @@ class Compiler
unsigned fgBBcountAtCodegen; // # of BBs in the method at the start of codegen
jitstd::vector<BasicBlock*>* fgBBOrder; // ordered vector of BBs
#endif
unsigned fgBBNumMin; // The min bbNum that has been assigned to basic blocks
unsigned fgBBNumMax; // The max bbNum that has been assigned to basic blocks
unsigned fgDomBBcount; // # of BBs for which we have dominator and reachability information
BasicBlock** fgBBReversePostorder; // Blocks in reverse postorder
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/jit/fgbasic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ void Compiler::fgInit()
fgBBOrder = nullptr;
#endif // DEBUG

fgBBNumMin = compIsForInlining() ? impInlineRoot()->fgBBNumMax + 1 : 1;
fgBBNumMax = 0;
fgEdgeCount = 0;
fgDomBBcount = 0;
Expand Down Expand Up @@ -5440,7 +5439,7 @@ bool Compiler::fgRenumberBlocks()

bool renumbered = false;
bool newMaxBBNum = false;
unsigned num = fgBBNumMin;
unsigned num = 1;

for (BasicBlock* block : Blocks())
{
Expand All @@ -5456,7 +5455,7 @@ bool Compiler::fgRenumberBlocks()
if (block->bbNext == nullptr)
{
fgLastBB = block;
fgBBcount = num - fgBBNumMin + 1;
fgBBcount = num;
if (fgBBNumMax != num)
{
fgBBNumMax = num;
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/jit/fgdiagnostic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
// that size, even though it means allocating a block map possibly much bigger than what's required for just
// the inlinee blocks.

unsigned blkMapSize = 1 + impInlineRoot()->fgBBNumMax;
unsigned blkMapSize = 1 + fgBBNumMax;
unsigned blockOrdinal = 1;
unsigned* blkMap = new (this, CMK_DebugOnly) unsigned[blkMapSize];
memset(blkMap, 0, sizeof(unsigned) * blkMapSize);
Expand Down Expand Up @@ -1830,7 +1830,7 @@ void Compiler::fgDispDoms()
void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 */)
{
const unsigned __int64 flags = block->bbFlags;
unsigned bbNumMax = impInlineRoot()->fgBBNumMax;
unsigned bbNumMax = fgBBNumMax;
int maxBlockNumWidth = CountDigits(bbNumMax);
maxBlockNumWidth = max(maxBlockNumWidth, 2);
int blockNumWidth = CountDigits(block->bbNum);
Expand Down Expand Up @@ -2261,7 +2261,7 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock,
ibcColWidth = max(ibcColWidth, 3) + 1; // + 1 for the leading space
}

unsigned bbNumMax = impInlineRoot()->fgBBNumMax;
unsigned bbNumMax = fgBBNumMax;
int maxBlockNumWidth = CountDigits(bbNumMax);
maxBlockNumWidth = max(maxBlockNumWidth, 2);
int padWidth = maxBlockNumWidth - 2; // Account for functions with a large number of blocks.
Expand Down Expand Up @@ -3635,7 +3635,7 @@ void Compiler::fgDebugCheckBlockLinks()
{
// Create a set with all the successors. Don't use BlockSet, so we don't need to worry
// about the BlockSet epoch.
BitVecTraits bitVecTraits(impInlineRoot()->fgBBNumMax + 1, this);
BitVecTraits bitVecTraits(fgBBNumMax + 1, this);
BitVec succBlocks(BitVecOps::MakeEmpty(&bitVecTraits));
for (BasicBlock* const bTarget : block->SwitchTargets())
{
Expand Down Expand Up @@ -4362,7 +4362,7 @@ void Compiler::fgDebugCheckLoopTable()
// `blockNumMap[bbNum] == 0` if the `bbNum` block was deleted and blocks haven't been renumbered since
// the deletion.

unsigned bbNumMax = impInlineRoot()->fgBBNumMax;
unsigned bbNumMax = fgBBNumMax;

// blockNumMap[old block number] => new block number
size_t blockNumBytes = (bbNumMax + 1) * sizeof(unsigned);
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/fgflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,7 @@ Compiler::SwitchUniqueSuccSet Compiler::GetDescriptorForSwitch(BasicBlock* switc
// can create a new epoch, thus invalidating all existing BlockSet objects, such as
// reachability information stored in the blocks. To avoid that, we just use a local BitVec.

unsigned bbNumMax = impInlineRoot()->fgBBNumMax;
BitVecTraits blockVecTraits(bbNumMax + 1, this);
BitVecTraits blockVecTraits(fgBBNumMax + 1, this);
BitVec uniqueSuccBlocks(BitVecOps::MakeEmpty(&blockVecTraits));
for (BasicBlock* const targ : switchBlk->SwitchTargets())
{
Expand Down
119 changes: 60 additions & 59 deletions src/coreclr/jit/fginline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1305,8 +1305,9 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
}
#endif // DEBUG

BasicBlock* topBlock = iciBlock;
BasicBlock* bottomBlock = nullptr;
BasicBlock* topBlock = iciBlock;
BasicBlock* bottomBlock = nullptr;
bool insertInlineeBlocks = true;

if (InlineeCompiler->fgBBcount == 1)
{
Expand Down Expand Up @@ -1356,85 +1357,85 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)

// Append statements to null out gc ref locals, if necessary.
fgInlineAppendStatements(pInlineInfo, iciBlock, stmtAfter);

goto _Done;
insertInlineeBlocks = false;
}
}

//
// ======= Inserting inlinee's basic blocks ===============
//
bottomBlock = fgSplitBlockAfterStatement(topBlock, stmtAfter);

//
// Set the try and handler index and fix the jump types of inlinee's blocks.
//
for (BasicBlock* const block : InlineeCompiler->Blocks())
if (insertInlineeBlocks)
{
noway_assert(!block->hasTryIndex());
noway_assert(!block->hasHndIndex());
block->copyEHRegion(iciBlock);
block->bbFlags |= iciBlock->bbFlags & BBF_BACKWARD_JUMP;
bottomBlock = fgSplitBlockAfterStatement(topBlock, stmtAfter);
unsigned const baseBBNum = fgBBNumMax;

DebugInfo di = iciStmt->GetDebugInfo().GetRoot();
if (di.IsValid())
{
block->bbCodeOffs = di.GetLocation().GetOffset();
block->bbCodeOffsEnd = block->bbCodeOffs + 1; // TODO: is code size of 1 some magic number for inlining?
}
else
//
// Set the try and handler index and fix the jump types of inlinee's blocks.
//
for (BasicBlock* const block : InlineeCompiler->Blocks())
{
block->bbCodeOffs = 0; // TODO: why not BAD_IL_OFFSET?
block->bbCodeOffsEnd = 0;
block->bbFlags |= BBF_INTERNAL;
}
noway_assert(!block->hasTryIndex());
noway_assert(!block->hasHndIndex());
block->copyEHRegion(iciBlock);
block->bbFlags |= iciBlock->bbFlags & BBF_BACKWARD_JUMP;

if (block->bbJumpKind == BBJ_RETURN)
{
noway_assert((block->bbFlags & BBF_HAS_JMP) == 0);
if (block->bbNext)
// Update block nums appropriately
//
block->bbNum += baseBBNum;
fgBBNumMax = max(block->bbNum, fgBBNumMax);

DebugInfo di = iciStmt->GetDebugInfo().GetRoot();
if (di.IsValid())
{
JITDUMP("\nConvert bbJumpKind of " FMT_BB " to BBJ_ALWAYS to bottomBlock " FMT_BB "\n", block->bbNum,
bottomBlock->bbNum);
block->bbJumpKind = BBJ_ALWAYS;
block->bbJumpDest = bottomBlock;
block->bbCodeOffs = di.GetLocation().GetOffset();
block->bbCodeOffsEnd = block->bbCodeOffs + 1; // TODO: is code size of 1 some magic number for inlining?
}
else
{
JITDUMP("\nConvert bbJumpKind of " FMT_BB " to BBJ_NONE\n", block->bbNum);
block->bbJumpKind = BBJ_NONE;
block->bbCodeOffs = 0; // TODO: why not BAD_IL_OFFSET?
block->bbCodeOffsEnd = 0;
block->bbFlags |= BBF_INTERNAL;
}

fgAddRefPred(bottomBlock, block);
}
}
if (block->bbJumpKind == BBJ_RETURN)
{
noway_assert((block->bbFlags & BBF_HAS_JMP) == 0);
if (block->bbNext)
{
JITDUMP("\nConvert bbJumpKind of " FMT_BB " to BBJ_ALWAYS to bottomBlock " FMT_BB "\n",
block->bbNum, bottomBlock->bbNum);
block->bbJumpKind = BBJ_ALWAYS;
block->bbJumpDest = bottomBlock;
}
else
{
JITDUMP("\nConvert bbJumpKind of " FMT_BB " to BBJ_NONE\n", block->bbNum);
block->bbJumpKind = BBJ_NONE;
}

// Inlinee's top block will have an artificial ref count. Remove.
assert(InlineeCompiler->fgFirstBB->bbRefs > 0);
InlineeCompiler->fgFirstBB->bbRefs--;
fgAddRefPred(bottomBlock, block);
}
}

// Insert inlinee's blocks into inliner's block list.
topBlock->setNext(InlineeCompiler->fgFirstBB);
fgRemoveRefPred(bottomBlock, topBlock);
fgAddRefPred(InlineeCompiler->fgFirstBB, topBlock);
InlineeCompiler->fgLastBB->setNext(bottomBlock);
// Inlinee's top block will have an artificial ref count. Remove.
assert(InlineeCompiler->fgFirstBB->bbRefs > 0);
InlineeCompiler->fgFirstBB->bbRefs--;

//
// Add inlinee's block count to inliner's.
//
fgBBcount += InlineeCompiler->fgBBcount;
// Insert inlinee's blocks into inliner's block list.
topBlock->setNext(InlineeCompiler->fgFirstBB);
fgRemoveRefPred(bottomBlock, topBlock);
fgAddRefPred(InlineeCompiler->fgFirstBB, topBlock);
InlineeCompiler->fgLastBB->setNext(bottomBlock);

// Append statements to null out gc ref locals, if necessary.
fgInlineAppendStatements(pInlineInfo, bottomBlock, nullptr);
//
// Add inlinee's block count to inliner's.
//
fgBBcount += InlineeCompiler->fgBBcount;

#ifdef DEBUG
if (verbose)
{
fgDispBasicBlocks(InlineeCompiler->fgFirstBB, InlineeCompiler->fgLastBB, true);
// Append statements to null out gc ref locals, if necessary.
fgInlineAppendStatements(pInlineInfo, bottomBlock, nullptr);
JITDUMPEXEC(fgDispBasicBlocks(InlineeCompiler->fgFirstBB, InlineeCompiler->fgLastBB, true));
}
#endif // DEBUG

_Done:

//
// At this point, we have successully inserted inlinee's code.
Expand Down
11 changes: 7 additions & 4 deletions src/coreclr/jit/fgopt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,16 @@ void Compiler::fgComputeEnterBlocksSet()
assert(fgFirstBB->bbNum == 1);

/* Also 'or' in the handler basic blocks */
for (EHblkDsc* const HBtab : EHClauses(this))
if (!compIsForInlining())
{
if (HBtab->HasFilter())
for (EHblkDsc* const HBtab : EHClauses(this))
{
BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdFilter->bbNum);
if (HBtab->HasFilter())
{
BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdFilter->bbNum);
}
BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdHndBeg->bbNum);
}
BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdHndBeg->bbNum);
}

#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
Expand Down
Loading

0 comments on commit bd63baa

Please sign in to comment.