Skip to content

Commit e6428cb

Browse files
CopilotjakobbotschdhartglassMSFT
authored
JIT: bail on flipping post-layout JTRUE when reversal needs new IR (#127746)
`optOptimizePostLayout` runs after LSRA and called `gtReverseCond`, which can fall back to wrapping the operand in a fresh `GT_EQ(tree, 0)`. The new `GT_INT_CON` child is never inserted into the LIR, tripping `found use of a node that is not in the LIR sequence` in checked builds (reported by Fuzzlyn on linux-arm32). The fallback was also reached whenever LSRA inserted a `GT_COPY`/`GT_RELOAD` on top of the JTRUE operand, since the wrapper is not itself a comparison. ## Description - **`gentree.cpp` / `compiler.h`** — Extract `gtTryReverseCond(GenTree*) -> bool` that reverses compares, `JCC`/`SETCC`, `JCMP`/`JTEST`, and integral constants in place. `gtReverseCond` becomes a thin wrapper that adds the `GT_EQ(tree, 0)` fallback when in-place reversal isn't possible, preserving semantics for all existing callers. - **`optimizer.cpp`** — In `optOptimizePostLayout`, peel `GT_COPY`/`GT_RELOAD` off the `GT_JTRUE` operand via `gtSkipReloadOrCopy()`, then use `gtTryReverseCond` and skip flipping the block if it returns `false` rather than mutating the LIR. ```cpp GenTree* cond = test; if (test->OperIs(GT_JTRUE)) { cond = test->gtGetOp1()->gtSkipReloadOrCopy(); } if (!gtTryReverseCond(cond)) { continue; } ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jakobbotsch <7887810+jakobbotsch@users.noreply.github.com> Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com> Co-authored-by: dhartglassMSFT <248563697+dhartglassMSFT@users.noreply.github.com>
1 parent 710da3b commit e6428cb

3 files changed

Lines changed: 51 additions & 22 deletions

File tree

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3919,6 +3919,7 @@ class Compiler
39193919
bool gtComplexityExceeds(GenTree* tree, unsigned limit, TFunc getComplexity);
39203920

39213921
GenTree* gtReverseCond(GenTree* tree);
3922+
bool gtTryReverseCond(GenTree* tree);
39223923

39233924
static bool gtHasRef(GenTree* tree, unsigned lclNum);
39243925

src/coreclr/jit/gentree.cpp

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3717,12 +3717,19 @@ genTreeOps GenTree::SwapRelop(genTreeOps relop)
37173717
return swapOps[relop - GT_EQ];
37183718
}
37193719

3720-
/*****************************************************************************
3721-
*
3722-
* Reverse the meaning of the given test condition.
3723-
*/
3724-
3725-
GenTree* Compiler::gtReverseCond(GenTree* tree)
3720+
//------------------------------------------------------------------------
3721+
// gtTryReverseCond: Try to reverse the meaning of the given test condition
3722+
// in-place, without introducing any new IR nodes.
3723+
//
3724+
// Arguments:
3725+
// tree - The condition tree to reverse
3726+
//
3727+
// Return Value:
3728+
// True if the condition was reversed in-place. False if reversing the
3729+
// condition would require introducing a new node (e.g. wrapping the tree
3730+
// in a "tree == 0" comparison); in that case, the tree is not modified.
3731+
//
3732+
bool Compiler::gtTryReverseCond(GenTree* tree)
37263733
{
37273734
if (tree->OperIsCompare())
37283735
{
@@ -3736,23 +3743,45 @@ GenTree* Compiler::gtReverseCond(GenTree* tree)
37363743
{
37373744
tree->gtFlags ^= GTF_RELOP_NAN_UN;
37383745
}
3746+
return true;
37393747
}
3740-
else if (tree->OperIs(GT_JCC, GT_SETCC))
3748+
if (tree->OperIs(GT_JCC, GT_SETCC))
37413749
{
37423750
GenTreeCC* cc = tree->AsCC();
37433751
cc->gtCondition = GenCondition::Reverse(cc->gtCondition);
3752+
return true;
37443753
}
3745-
else if (tree->OperIs(GT_JCMP, GT_JTEST))
3754+
if (tree->OperIs(GT_JCMP, GT_JTEST))
37463755
{
37473756
GenTreeOpCC* opCC = tree->AsOpCC();
37483757
opCC->gtCondition = GenCondition::Reverse(opCC->gtCondition);
3758+
return true;
37493759
}
3750-
else if (tree->IsIntegralConst())
3760+
if (tree->IsIntegralConst())
37513761
{
37523762
GenTreeIntConCommon* con = tree->AsIntConCommon();
37533763
con->SetIntegralValue(con->IsIntegralConst(0) ? 1 : 0);
3764+
return true;
37543765
}
3755-
else
3766+
3767+
return false;
3768+
}
3769+
3770+
//------------------------------------------------------------------------
3771+
// gtReverseCond: Reverse the meaning of the given test condition.
3772+
//
3773+
// Arguments:
3774+
// tree - The condition tree to reverse
3775+
//
3776+
// Return Value:
3777+
// The reversed condition tree. This is normally the same node as the
3778+
// input tree, with its operator (or condition) reversed in-place. If the
3779+
// tree cannot be reversed in-place, a new GT_EQ node comparing the tree
3780+
// to zero is returned instead.
3781+
//
3782+
GenTree* Compiler::gtReverseCond(GenTree* tree)
3783+
{
3784+
if (!gtTryReverseCond(tree))
37563785
{
37573786
tree = gtNewOperNode(GT_EQ, TYP_INT, tree, gtNewZeroConNode(TYP_INT));
37583787
}

src/coreclr/jit/optimizer.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,22 +2426,21 @@ PhaseStatus Compiler::optOptimizePostLayout()
24262426
GenTree* const test = block->lastNode();
24272427
assert(test->OperIsConditionalJump());
24282428

2429+
// Try to reverse the condition in-place. We are running after LSRA, so we cannot
2430+
// introduce new IR nodes here. If the condition cannot be reversed in-place, bail
2431+
// on flipping for this block.
2432+
//
2433+
// For GT_JTRUE the operand may have a GT_COPY/GT_RELOAD inserted by LSRA on top of
2434+
// the actual condition node, so skip those to find the underlying condition.
2435+
GenTree* cond = test;
24292436
if (test->OperIs(GT_JTRUE))
24302437
{
2431-
// Flip GT_JTRUE node's conditional operand, and handle any new nodes this may introduce
2432-
GenTree* const cond = test->gtGetOp1();
2433-
GenTree* const newCond = gtReverseCond(cond);
2434-
if (cond != newCond)
2435-
{
2436-
LIR::AsRange(block).InsertAfter(cond, newCond);
2437-
test->AsUnOp()->gtOp1 = newCond;
2438-
}
2438+
cond = test->gtGetOp1()->gtSkipReloadOrCopy();
24392439
}
2440-
else
2440+
2441+
if (!gtTryReverseCond(cond))
24412442
{
2442-
// gtReverseCond can handle other conditional jumps without introducing a new node
2443-
GenTree* const cond = gtReverseCond(test);
2444-
assert(cond == test);
2443+
continue;
24452444
}
24462445

24472446
FlowEdge* const oldTrueEdge = block->GetTrueEdge();

0 commit comments

Comments
 (0)