Skip to content

Commit 4d629d7

Browse files
Fix tree threading in fgOptimizeBranch (#51422)
In case `fgOptimizeBranch` gets far enough to calculate the cost of statements it is considering duplicating, then it calls `gtPrepareCost` => `gtSetEvalOrder` to compute a statement's costs. Unfortunately, that can reverse operands (and in this case, did). `fgOptimizeBranch` is called only by `fgReorderBlocks` which is called by `fgInsertGCPolls`, late enough that the tree is already threaded with gtNext/gtPrev links. (The test case inserted multiple GC polls for SuppressGCTransitionAttribute code.) When the operands of the tree were swapped, nobody fixed up these links. The only other caller of `fgReorderBlocks` is `optOptimizeLayout`, which comes before the trees are threaded. The solution is to simply call `fgSetStmtSeq` if needed. No SPMI x86, x64 asm diffs. Fixes #51293, a failure in JIT/Performance/CodeQuality/Burgers/Burgers/Burgers.sh on Linux arm32 under JitStress=1. There is IBC data, and setting `COMPlus_JitDisablePgo=1` also fixes the failure.
1 parent 4573751 commit 4d629d7

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

src/coreclr/jit/fgopt.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3376,11 +3376,17 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
33763376
unsigned estDupCostSz = 0;
33773377
for (Statement* stmt : bDest->Statements())
33783378
{
3379-
GenTree* expr = stmt->GetRootNode();
3379+
// We want to compute the costs of the statement. Unfortunately, gtPrepareCost() / gtSetStmtInfo()
3380+
// call gtSetEvalOrder(), which can reorder nodes. If it does so, we need to re-thread the gtNext/gtPrev
3381+
// links. We don't know if it does or doesn't reorder nodes, so we end up always re-threading the links.
33803382

3381-
/* We call gtPrepareCost to measure the cost of duplicating this tree */
3382-
gtPrepareCost(expr);
3383+
gtSetStmtInfo(stmt);
3384+
if (fgStmtListThreaded)
3385+
{
3386+
fgSetStmtSeq(stmt);
3387+
}
33833388

3389+
GenTree* expr = stmt->GetRootNode();
33843390
estDupCostSz += expr->GetCostSz();
33853391
}
33863392

0 commit comments

Comments
 (0)