Skip to content

Commit f2f734a

Browse files
authored
JIT: Slightly refactor handling of conditions/conditional branches in the backend (#82020)
Slightly refactor how compares are represented and processed in the backend in preparation of lifting the remaining cmov restrictions. - Change all compare nodes to always produce values and never set flags. That is, GT_EQ/GT_NE/GT_LT/GT_LE/GT_GE/GT_GT/GT_TEST_EQ/GT_TEST_NE now are all "normal" nodes, always. - Stop optimizing compares by looking for a user JTRUE node. Instead, we optimize the compare when we see it (for example, EQ/NE(AND(x, y), 0) is turned into GT_TEST_EQ/GT_TEST_NE(x, y)), but we do not change the user JTRUE node to a JCC node. We now never create SETCC nodes when we see the compare, and we always turn JTRUE nodes into JCC nodes with a proper condition. - Handle the transformation from JTRUE to JCC when we actually get to JTRUE. The transformation that now happens is: 1. The compare node is turned into a GT_CMP (for GT_EQ to GT_GT) or GT_TEST node (for GT_TEST_EQ/GT_TEST_NE) node. These nodes are always void-typed and do not produce values; they only set the CPU flags, and correspond directly to low-level target instructions (like cmp/test). 2. The JTRUE is turned into JCC with the condition from the compare. For arm64/xarch, we handle optimizing EQ/NE(node_that_sets_zero_flag, 0) at this stage too now. - Introduce new xarch backend-only GT_BITTEST_EQ/GT_BITTEST_NE that semantically correspond to (x & (1 << y)) != 0. The corresponding "set flags" node is GT_BT and already exists. We would previously transform (x & (1 << y)) != 0 into GT_BT + GT_SETCC (when the user was not a JTRUE). This is now transformed into GT_BITTEST_EQ/GT_BITTEST_NE instead, which are completely normal nodes. The processing of JTRUE handles the transformation into BT + JCC. - Change some of the emitter compare peepholes to work with this scheme. This requires some lookahead when we see them, but allows us to get rid of an "ugly" flag. - Allow liveness to DCE GT_CMP/GT_TEST when they have had GTF_SET_FLAGS unset (due to FG optimizations). We already allowed this for GT_EQ, so this showed up as some regressions. Some example LIR diffs: ```diff Generating: N033 (???,???) [000136] ----------- IL_OFFSET void INLRT @ 0x00A[E-] REG NA Generating: N035 ( 1, 1) [000009] ----------- t9 = LCL_VAR int V02 loc1 u:1 ecx REG ecx $141 Generating: N037 ( 1, 1) [000011] -c--------- t11 = CNS_INT int 16 REG NA $42 ┌──▌ t9 int ├──▌ t11 int - Generating: N039 ( 3, 3) [000012] N------N-U- ▌ GE void REG NA $142 + Generating: N039 ( 3, 3) [000012] -------N-U- ▌ CMP void REG NA IN0005: cmp ecx, 16 - Generating: N041 ( 5, 5) [000013] ----------- ▌ JTRUE void REG NA $VN.Void + Generating: N041 (???,???) [000144] ----------- JCC void cond=UGE REG NA IN0006: jae L_M20112_BB03 ┌──▌ t112 ubyte ├──▌ t54 int - Generating: N061 ( 22, 14) [000060] ---XGO----- ▌ BT void REG NA + Generating: N061 ( 22, 14) [000060] ---XGO----- t60 = ▌ BITTEST_NE int REG eax IN0009: bt eax, esi - Generating: N063 (???,???) [000144] ----------- t144 = SETCC int cond=C REG eax IN000a: setb al IN000b: movzx eax, al - ┌──▌ t144 int + ┌──▌ t60 int Generating: N065 ( 23, 15) [000061] ---XGO----- ▌ RETURN int REG NA <l:$1c8, c:$1c7> ```
1 parent 4f19d17 commit f2f734a

23 files changed

+344
-321
lines changed

src/coreclr/jit/codegen.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
902902

903903
void genCompareFloat(GenTree* treeNode);
904904
void genCompareInt(GenTree* treeNode);
905+
#ifdef TARGET_XARCH
906+
bool genCanAvoidEmittingCompareAgainstZero(GenTree* tree, var_types opType);
907+
GenTreeCC* genTryFindFlagsConsumer(GenTree* flagsProducer);
908+
#endif
905909

906910
#ifdef FEATURE_SIMD
907911
#ifdef TARGET_ARM64
@@ -1077,7 +1081,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
10771081

10781082
#ifdef TARGET_XARCH
10791083
void genCodeForShiftRMW(GenTreeStoreInd* storeInd);
1080-
void genCodeForBT(GenTreeOp* bt);
10811084
#endif // TARGET_XARCH
10821085

10831086
void genCodeForCast(GenTreeOp* tree);
@@ -1192,7 +1195,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
11921195
void genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackArgBytes));
11931196
void genJmpMethod(GenTree* jmp);
11941197
BasicBlock* genCallFinally(BasicBlock* block);
1195-
void genCodeForJumpTrue(GenTreeOp* jtrue);
11961198
#if defined(TARGET_LOONGARCH64)
11971199
// TODO: refactor for LA.
11981200
void genCodeForJumpCompare(GenTreeOp* tree);

src/coreclr/jit/codegenarm.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,13 +1257,9 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
12571257
regNumber targetReg = tree->GetRegNum();
12581258
emitter* emit = GetEmitter();
12591259

1260-
genConsumeIfReg(op1);
1261-
genConsumeIfReg(op2);
1262-
12631260
if (varTypeIsFloating(op1Type))
12641261
{
12651262
assert(op1Type == op2Type);
1266-
assert(!tree->OperIs(GT_CMP));
12671263
emit->emitInsBinary(INS_vcmp, emitTypeSize(op1Type), op1, op2);
12681264
// vmrs with register 0xf has special meaning of transferring flags
12691265
emit->emitIns_R(INS_vmrs, EA_4BYTE, REG_R15);

src/coreclr/jit/codegenarm64.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4578,7 +4578,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
45784578
// We don't support swapping op1 and op2 to generate cmp reg, imm
45794579
assert(!op1->isContainedIntOrIImmed());
45804580

4581-
instruction ins = tree->OperIs(GT_TEST_EQ, GT_TEST_NE) ? INS_tst : INS_cmp;
4581+
instruction ins = tree->OperIs(GT_TEST_EQ, GT_TEST_NE, GT_TEST) ? INS_tst : INS_cmp;
45824582

45834583
if (op2->isContainedIntOrIImmed())
45844584
{

src/coreclr/jit/codegenarmarch.cpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -349,16 +349,11 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
349349
case GT_LE:
350350
case GT_GE:
351351
case GT_GT:
352-
case GT_CMP:
353-
#ifdef TARGET_ARM64
354-
case GT_TEST_EQ:
355352
case GT_TEST_NE:
356-
// On ARM64 genCodeForCompare does not consume its own operands because
357-
// genCodeForBinary also has this behavior and it can end up calling
358-
// genCodeForCompare when generating compare chains for GT_AND.
359-
// Thus, we must do it here.
353+
case GT_TEST_EQ:
354+
case GT_CMP:
355+
case GT_TEST:
360356
genConsumeOperands(treeNode->AsOp());
361-
#endif // TARGET_ARM64
362357
genCodeForCompare(treeNode->AsOp());
363358
break;
364359

@@ -368,10 +363,6 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
368363
break;
369364
#endif
370365

371-
case GT_JTRUE:
372-
genCodeForJumpTrue(treeNode->AsOp());
373-
break;
374-
375366
#ifdef TARGET_ARM64
376367
case GT_JCMP:
377368
genCodeForJumpCompare(treeNode->AsOp());

src/coreclr/jit/codegenlinear.cpp

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2588,51 +2588,6 @@ void CodeGen::genStoreLongLclVar(GenTree* treeNode)
25882588
#endif // !defined(TARGET_64BIT)
25892589

25902590
#ifndef TARGET_LOONGARCH64
2591-
//------------------------------------------------------------------------
2592-
// genCodeForJumpTrue: Generate code for a GT_JTRUE node.
2593-
//
2594-
// Arguments:
2595-
// jtrue - The node
2596-
//
2597-
void CodeGen::genCodeForJumpTrue(GenTreeOp* jtrue)
2598-
{
2599-
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
2600-
assert(jtrue->OperIs(GT_JTRUE));
2601-
2602-
GenTreeOp* relop = jtrue->gtGetOp1()->AsOp();
2603-
GenCondition condition = GenCondition::FromRelop(relop);
2604-
2605-
if (condition.PreferSwap())
2606-
{
2607-
condition = GenCondition::Swap(condition);
2608-
}
2609-
2610-
#if defined(TARGET_XARCH)
2611-
if ((condition.GetCode() == GenCondition::FNEU) &&
2612-
(relop->gtGetOp1()->GetRegNum() == relop->gtGetOp2()->GetRegNum()) &&
2613-
!relop->gtGetOp1()->isUsedFromSpillTemp() && !relop->gtGetOp2()->isUsedFromSpillTemp())
2614-
{
2615-
// For floating point, `x != x` is a common way of
2616-
// checking for NaN. So, in the case where both
2617-
// operands are the same, we can optimize codegen
2618-
// to only do a single check.
2619-
2620-
condition = GenCondition(GenCondition::P);
2621-
}
2622-
2623-
if (relop->MarkedForSignJumpOpt())
2624-
{
2625-
// If relop was previously marked for a signed jump check optimization because of SF flag
2626-
// reuse, replace jge/jl with jns/js.
2627-
2628-
assert(relop->OperGet() == GT_LT || relop->OperGet() == GT_GE);
2629-
condition = (relop->OperGet() == GT_LT) ? GenCondition(GenCondition::S) : GenCondition(GenCondition::NS);
2630-
}
2631-
2632-
#endif
2633-
2634-
inst_JCC(condition, compiler->compCurBB->bbJumpDest);
2635-
}
26362591

26372592
//------------------------------------------------------------------------
26382593
// genCodeForJcc: Generate code for a GT_JCC node.

0 commit comments

Comments
 (0)