Skip to content

Commit b82dc92

Browse files
authored
JIT: slightly more aggressive tail duplication (#61179)
Catch patterns like the one in #37904 where a trinary compare feeds a binary compare.
1 parent b837efa commit b82dc92

File tree

1 file changed

+137
-21
lines changed

1 file changed

+137
-21
lines changed

src/coreclr/jit/fgopt.cpp

Lines changed: 137 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3220,7 +3220,9 @@ bool Compiler::fgBlockIsGoodTailDuplicationCandidate(BasicBlock* target, unsigne
32203220
{
32213221
*lclNum = BAD_VAR_NUM;
32223222

3223-
// Here we are looking for blocks with a single statement feeding a conditional branch.
3223+
// Here we are looking for small blocks where a local live-into the block
3224+
// ultimately feeds a simple conditional branch.
3225+
//
32243226
// These blocks are small, and when duplicated onto the tail of blocks that end in
32253227
// assignments, there is a high probability of the branch completely going away.
32263228
//
@@ -3237,22 +3239,27 @@ bool Compiler::fgBlockIsGoodTailDuplicationCandidate(BasicBlock* target, unsigne
32373239
return false;
32383240
}
32393241

3240-
Statement* stmt = target->FirstNonPhiDef();
3242+
Statement* const lastStmt = target->lastStmt();
3243+
Statement* const firstStmt = target->FirstNonPhiDef();
32413244

3242-
if (stmt != target->lastStmt())
3245+
// We currently allow just one statement aside from the branch.
3246+
//
3247+
if ((firstStmt != lastStmt) && (firstStmt != lastStmt->GetPrevStmt()))
32433248
{
32443249
return false;
32453250
}
32463251

3247-
GenTree* tree = stmt->GetRootNode();
3252+
// Verify the branch is just a simple local compare.
3253+
//
3254+
GenTree* const lastTree = lastStmt->GetRootNode();
32483255

3249-
if (tree->gtOper != GT_JTRUE)
3256+
if (lastTree->gtOper != GT_JTRUE)
32503257
{
32513258
return false;
32523259
}
32533260

32543261
// must be some kind of relational operator
3255-
GenTree* const cond = tree->AsOp()->gtOp1;
3262+
GenTree* const cond = lastTree->AsOp()->gtOp1;
32563263
if (!cond->OperIsCompare())
32573264
{
32583265
return false;
@@ -3314,6 +3321,109 @@ bool Compiler::fgBlockIsGoodTailDuplicationCandidate(BasicBlock* target, unsigne
33143321
return false;
33153322
}
33163323

3324+
// If there's no second statement, we're good.
3325+
//
3326+
if (firstStmt == lastStmt)
3327+
{
3328+
return true;
3329+
}
3330+
3331+
// Otherwise check the first stmt.
3332+
// Verify the branch is just a simple local compare.
3333+
//
3334+
GenTree* const firstTree = firstStmt->GetRootNode();
3335+
3336+
if (firstTree->gtOper != GT_ASG)
3337+
{
3338+
return false;
3339+
}
3340+
3341+
GenTree* const lhs = firstTree->AsOp()->gtOp1;
3342+
if (!lhs->OperIs(GT_LCL_VAR))
3343+
{
3344+
return false;
3345+
}
3346+
3347+
const unsigned lhsLcl = lhs->AsLclVarCommon()->GetLclNum();
3348+
if (lhsLcl != *lclNum)
3349+
{
3350+
return false;
3351+
}
3352+
3353+
// Could allow unary here too...
3354+
//
3355+
GenTree* const rhs = firstTree->AsOp()->gtOp2;
3356+
if (!rhs->OperIsBinary())
3357+
{
3358+
return false;
3359+
}
3360+
3361+
// op1 must be some combinations of casts of local or constant
3362+
// (or unary)
3363+
op1 = rhs->AsOp()->gtOp1;
3364+
while (op1->gtOper == GT_CAST)
3365+
{
3366+
op1 = op1->AsOp()->gtOp1;
3367+
}
3368+
3369+
if (!op1->IsLocal() && !op1->OperIsConst())
3370+
{
3371+
return false;
3372+
}
3373+
3374+
// op2 must be some combinations of casts of local or constant
3375+
// (or unary)
3376+
op2 = rhs->AsOp()->gtOp2;
3377+
3378+
// A binop may not actually have an op2.
3379+
//
3380+
if (op2 == nullptr)
3381+
{
3382+
return false;
3383+
}
3384+
3385+
while (op2->gtOper == GT_CAST)
3386+
{
3387+
op2 = op2->AsOp()->gtOp1;
3388+
}
3389+
3390+
if (!op2->IsLocal() && !op2->OperIsConst())
3391+
{
3392+
return false;
3393+
}
3394+
3395+
// Tree must have one constant and one local, or be comparing
3396+
// the same local to itself.
3397+
lcl1 = BAD_VAR_NUM;
3398+
lcl2 = BAD_VAR_NUM;
3399+
3400+
if (op1->IsLocal())
3401+
{
3402+
lcl1 = op1->AsLclVarCommon()->GetLclNum();
3403+
}
3404+
3405+
if (op2->IsLocal())
3406+
{
3407+
lcl2 = op2->AsLclVarCommon()->GetLclNum();
3408+
}
3409+
3410+
if ((lcl1 != BAD_VAR_NUM) && op2->OperIsConst())
3411+
{
3412+
*lclNum = lcl1;
3413+
}
3414+
else if ((lcl2 != BAD_VAR_NUM) && op1->OperIsConst())
3415+
{
3416+
*lclNum = lcl2;
3417+
}
3418+
else if ((lcl1 != BAD_VAR_NUM) && (lcl1 == lcl2))
3419+
{
3420+
*lclNum = lcl1;
3421+
}
3422+
else
3423+
{
3424+
return false;
3425+
}
3426+
33173427
return true;
33183428
}
33193429

@@ -3333,6 +3443,8 @@ bool Compiler::fgBlockIsGoodTailDuplicationCandidate(BasicBlock* target, unsigne
33333443
//
33343444
bool Compiler::fgOptimizeUncondBranchToSimpleCond(BasicBlock* block, BasicBlock* target)
33353445
{
3446+
JITDUMP("Considering uncond to cond " FMT_BB " -> " FMT_BB "\n", block->bbNum, target->bbNum);
3447+
33363448
if (!BasicBlock::sameEHRegion(block, target))
33373449
{
33383450
return false;
@@ -3360,39 +3472,43 @@ bool Compiler::fgOptimizeUncondBranchToSimpleCond(BasicBlock* block, BasicBlock*
33603472
// backend always calls `fgUpdateFlowGraph` with `doTailDuplication` set to false.
33613473
assert(!block->IsLIR());
33623474

3363-
Statement* stmt = target->FirstNonPhiDef();
3364-
assert(stmt == target->lastStmt());
3365-
33663475
// Duplicate the target block at the end of this block
3367-
GenTree* cloned = gtCloneExpr(stmt->GetRootNode());
3368-
noway_assert(cloned);
3369-
Statement* jmpStmt = gtNewStmt(cloned);
3476+
//
3477+
for (Statement* stmt : target->NonPhiStatements())
3478+
{
3479+
GenTree* clone = gtCloneExpr(stmt->GetRootNode());
3480+
noway_assert(clone);
3481+
Statement* cloneStmt = gtNewStmt(clone);
33703482

3483+
if (fgStmtListThreaded)
3484+
{
3485+
gtSetStmtInfo(cloneStmt);
3486+
}
3487+
3488+
fgInsertStmtAtEnd(block, cloneStmt);
3489+
}
3490+
3491+
// Fix up block's flow
3492+
//
33713493
block->bbJumpKind = BBJ_COND;
33723494
block->bbJumpDest = target->bbJumpDest;
33733495
fgAddRefPred(block->bbJumpDest, block);
33743496
fgRemoveRefPred(target, block);
33753497

33763498
// add an unconditional block after this block to jump to the target block's fallthrough block
3499+
//
33773500
BasicBlock* next = fgNewBBafter(BBJ_ALWAYS, block, true);
33783501

33793502
// The new block 'next' will inherit its weight from 'block'
3503+
//
33803504
next->inheritWeight(block);
33813505
next->bbJumpDest = target->bbNext;
33823506
fgAddRefPred(next, block);
33833507
fgAddRefPred(next->bbJumpDest, next);
33843508

33853509
JITDUMP("fgOptimizeUncondBranchToSimpleCond(from " FMT_BB " to cond " FMT_BB "), created new uncond " FMT_BB "\n",
33863510
block->bbNum, target->bbNum, next->bbNum);
3387-
JITDUMP(" expecting opts to key off V%02u, added cloned compare [%06u] to " FMT_BB "\n", lclNum,
3388-
dspTreeID(cloned), block->bbNum);
3389-
3390-
if (fgStmtListThreaded)
3391-
{
3392-
gtSetStmtInfo(jmpStmt);
3393-
}
3394-
3395-
fgInsertStmtAtEnd(block, jmpStmt);
3511+
JITDUMP(" expecting opts to key off V%02u in " FMT_BB "\n", lclNum, block->bbNum);
33963512

33973513
return true;
33983514
}

0 commit comments

Comments
 (0)