Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Remove optLoopCompactionFixupFallThrough #97670

Merged
merged 4 commits into from
Jan 30, 2024
Merged

JIT: Remove optLoopCompactionFixupFallThrough #97670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5f2a66
Add branch reversal before reordering
amanasifkhalid Jan 29, 2024
c907c8f
Remove loop fallthrough fixup logic
amanasifkhalid Jan 29, 2024
54aabcb
Remove optLoopCompactionFixupFallThrough
amanasifkhalid Jan 29, 2024
728f22f
Update comment
amanasifkhalid Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6779,11 +6779,10 @@ class Compiler

void optFindLoops();
bool optCanonicalizeLoops();
bool optCompactLoops();
bool optCompactLoop(FlowGraphNaturalLoop* loop);
void optCompactLoops();
void optCompactLoop(FlowGraphNaturalLoop* loop);
BasicBlock* optFindLoopCompactionInsertionPoint(FlowGraphNaturalLoop* loop, BasicBlock* top);
BasicBlock* optTryAdvanceLoopCompactionInsertionPoint(FlowGraphNaturalLoop* loop, BasicBlock* insertionPoint, BasicBlock* top, BasicBlock* bottom);
bool optLoopCompactionFixupFallThrough(BasicBlock* block, BasicBlock* newNext);
bool optCreatePreheader(FlowGraphNaturalLoop* loop);
void optSetPreheaderWeight(FlowGraphNaturalLoop* loop, BasicBlock* preheader);

Expand Down
28 changes: 22 additions & 6 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3491,19 +3491,35 @@ bool Compiler::fgReorderBlocks(bool useProfile)
}
}

// If we will be reordering blocks, re-establish implicit fallthrough for BBJ_COND blocks
// If we will be reordering blocks, ensure the false target of a BBJ_COND block is its next block
if (useProfile)
{
for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->Next())
{
if (block->KindIs(BBJ_COND) && !block->NextIs(block->GetFalseTarget()))
{
BasicBlock* jmpBlk = fgConnectFallThrough(block, block->GetFalseTarget());
assert(jmpBlk != nullptr);
assert(block->NextIs(jmpBlk));
if (block->CanRemoveJumpToTarget(block->GetTrueTarget(), this))
{
// Reverse the jump condition
GenTree* test = block->lastNode();
assert(test->OperIsConditionalJump());
test->AsOp()->gtOp1 = gtReverseCond(test->AsOp()->gtOp1);

BasicBlock* newFalseTarget = block->GetTrueTarget();
BasicBlock* newTrueTarget = block->GetFalseTarget();
block->SetTrueTarget(newTrueTarget);
block->SetFalseTarget(newFalseTarget);
assert(block->CanRemoveJumpToTarget(newFalseTarget, this));
}
else
{
BasicBlock* jmpBlk = fgConnectFallThrough(block, block->GetFalseTarget());
assert(jmpBlk != nullptr);
assert(block->NextIs(jmpBlk));

// Skip next block
block = jmpBlk;
// Skip next block
block = jmpBlk;
}
}
}
}
Expand Down
122 changes: 5 additions & 117 deletions src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2671,12 +2671,7 @@ void Compiler::optFindLoops()
{
m_loops = FlowGraphNaturalLoops::Find(m_dfsTree);

if (optCompactLoops())
{
fgInvalidateDfsTree();
m_dfsTree = fgComputeDfs();
m_loops = FlowGraphNaturalLoops::Find(m_dfsTree);
}
optCompactLoops();

if (optCanonicalizeLoops())
{
Expand Down Expand Up @@ -2727,18 +2722,12 @@ bool Compiler::optCanonicalizeLoops()
//-----------------------------------------------------------------------------
// optCompactLoops: Compact loops to make their loop blocks lexical if possible.
//
// Returns:
// True if the flow graph was changed.
//
bool Compiler::optCompactLoops()
void Compiler::optCompactLoops()
{
bool changed = false;
for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder())
{
changed |= optCompactLoop(loop);
optCompactLoop(loop);
}

return changed;
}

//-----------------------------------------------------------------------------
Expand All @@ -2747,18 +2736,14 @@ bool Compiler::optCompactLoops()
// Parameters:
// loop - The loop
//
// Returns:
// True if the flow graph was changed.
//
bool Compiler::optCompactLoop(FlowGraphNaturalLoop* loop)
void Compiler::optCompactLoop(FlowGraphNaturalLoop* loop)
{
BasicBlock* insertionPoint = nullptr;

BasicBlock* top = loop->GetLexicallyTopMostBlock();
unsigned numLoopBlocks = loop->NumLoopBlocks();

BasicBlock* cur = top;
bool changedFlowGraph = false;
BasicBlock* cur = top;
while (numLoopBlocks > 0)
{
if (loop->ContainsBlock(cur))
Expand Down Expand Up @@ -2813,19 +2798,11 @@ bool Compiler::optCompactLoop(FlowGraphNaturalLoop* loop)
fgMoveBlocksAfter(cur, lastNonLoopBlock, insertionPoint);
ehUpdateLastBlocks(insertionPoint, lastNonLoopBlock);

// Apply any adjustments needed for fallthrough at the boundaries of the moved region.
changedFlowGraph |= optLoopCompactionFixupFallThrough(insertionPoint, cur);
changedFlowGraph |= optLoopCompactionFixupFallThrough(lastNonLoopBlock, moveBefore);
// Also apply any adjustments needed where the blocks were snipped out of the loop.
changedFlowGraph |= optLoopCompactionFixupFallThrough(previous, nextLoopBlock);

// Update insertionPoint for the next insertion.
insertionPoint = lastNonLoopBlock;

cur = nextLoopBlock;
}

return changedFlowGraph;
}

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -2927,95 +2904,6 @@ BasicBlock* Compiler::optTryAdvanceLoopCompactionInsertionPoint(FlowGraphNatural
: newInsertionPoint;
}

//-----------------------------------------------------------------------------
// optLoopCompactionFixupFallThrough: Fix up fallthrough introduced due to
// moving a range of blocks.
//
// Parameters:
// block - Block that may have fallthrough
// newNext - The new block that was the fallthrough block
//
// Returns:
// True if the flow graph was changed by this function.
//
bool Compiler::optLoopCompactionFixupFallThrough(BasicBlock* block, BasicBlock* newNext)
{
assert(block->NextIs(newNext));
bool changed = false;

if (block->KindIs(BBJ_COND) && block->TrueTargetIs(newNext))
{
// Reverse the jump condition
GenTree* test = block->lastNode();
noway_assert(test->OperIsConditionalJump());

if (test->OperGet() == GT_JTRUE)
{
GenTree* cond = gtReverseCond(test->AsOp()->gtOp1);
assert(cond == test->AsOp()->gtOp1); // Ensure `gtReverseCond` did not create a new node.
test->AsOp()->gtOp1 = cond;
}
else
{
gtReverseCond(test);
}

// Redirect the Conditional JUMP to go to `oldNext`
block->SetTrueTarget(block->GetFalseTarget());
block->SetFalseTarget(newNext);
changed = true;
}
else if (block->KindIs(BBJ_ALWAYS) && block->TargetIs(newNext))
{
// If block is newNext's only predecessor, move the IR from block to newNext,
// but keep the now-empty block around.
//
// We move the IR because loop recognition has a very limited search capability and
// won't walk from one block's statements to another, even if the blocks form
// a linear chain. So this IR move enhances counted loop recognition.
//
// The logic here is essentially echoing fgCompactBlocks... but we don't call
// that here because we don't want to delete block and do the necessary updates
// to all the other data in flight, and we'd also prefer that newNext be the
// survivor, not block.
//
if ((newNext->bbRefs == 1) && fgCanCompactBlocks(block, newNext))
{
JITDUMP("Moving stmts from " FMT_BB " to " FMT_BB "\n", block->bbNum, newNext->bbNum);
Statement* stmtList1 = block->firstStmt();
Statement* stmtList2 = newNext->firstStmt();

// Is there anything to move?
//
if (stmtList1 != nullptr)
{
// Append newNext stmts to block's stmts.
//
if (stmtList2 != nullptr)
{
Statement* stmtLast1 = block->lastStmt();
Statement* stmtLast2 = newNext->lastStmt();

stmtLast1->SetNextStmt(stmtList2);
stmtList2->SetPrevStmt(stmtLast1);
stmtList1->SetPrevStmt(stmtLast2);
}

// Move block's stmts to newNext
//
newNext->bbStmtList = stmtList1;
block->bbStmtList = nullptr;

// Update newNext's block flags
//
newNext->CopyFlags(block, BBF_COMPACT_UPD);
}
}
}

return changed;
}

//-----------------------------------------------------------------------------
// optCreatePreheader: Create (or find) a preheader for a natural loop.
//
Expand Down
Loading