Skip to content

Commit 5598dac

Browse files
amanasifkhalidamanasifkhalid
authored
JIT: Allow BBJ_COND false target to diverge from bbNext in layout optimization phase (dotnet#96609)
Next step for dotnet#93020. Working backwards through the JIT flowgraph phases, this change allows bbFalseTarget to diverge from bbNext in Compiler::optOptimizeLayout and onwards.
1 parent 3b58f10 commit 5598dac

19 files changed

+364
-286
lines changed

‎src/coreclr/jit/block.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -301,18 +301,20 @@ bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const
301301
}
302302

303303
//------------------------------------------------------------------------
304-
// CanRemoveJumpToFalseTarget: determine if jump to false target can be omitted
304+
// CanRemoveJumpToTarget: determine if jump to target can be omitted
305305
//
306306
// Arguments:
307+
// target - true/false target of BBJ_COND block
307308
// compiler - current compiler instance
308309
//
309310
// Returns:
310-
// true if block is a BBJ_COND that can fall into its false target
311+
// true if block is a BBJ_COND that can fall into target
311312
//
312-
bool BasicBlock::CanRemoveJumpToFalseTarget(Compiler* compiler) const
313+
bool BasicBlock::CanRemoveJumpToTarget(BasicBlock* target, Compiler* compiler) const
313314
{
314315
assert(KindIs(BBJ_COND));
315-
return NextIs(bbFalseTarget) && !hasAlign() && !compiler->fgInDifferentRegions(this, bbFalseTarget);
316+
assert(TrueTargetIs(target) || FalseTargetIs(target));
317+
return NextIs(target) && !compiler->fgInDifferentRegions(this, target);
316318
}
317319

318320
//------------------------------------------------------------------------
@@ -1170,7 +1172,7 @@ unsigned BasicBlock::NumSucc() const
11701172
return 1;
11711173

11721174
case BBJ_COND:
1173-
if (bbTarget == bbNext)
1175+
if (bbTrueTarget == bbFalseTarget)
11741176
{
11751177
return 1;
11761178
}
@@ -1295,7 +1297,7 @@ unsigned BasicBlock::NumSucc(Compiler* comp)
12951297
return 1;
12961298

12971299
case BBJ_COND:
1298-
if (bbTarget == bbNext)
1300+
if (bbTrueTarget == bbFalseTarget)
12991301
{
13001302
return 1;
13011303
}

‎src/coreclr/jit/block.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ struct BasicBlock : private LIR::Range
616616

617617
bool CanRemoveJumpToNext(Compiler* compiler) const;
618618

619-
bool CanRemoveJumpToFalseTarget(Compiler* compiler) const;
619+
bool CanRemoveJumpToTarget(BasicBlock* target, Compiler* compiler) const;
620620

621621
unsigned GetTargetOffs() const
622622
{
@@ -669,7 +669,6 @@ struct BasicBlock : private LIR::Range
669669
{
670670
assert(KindIs(BBJ_COND));
671671
assert(bbTrueTarget != nullptr);
672-
assert(target != nullptr);
673672
return (bbTrueTarget == target);
674673
}
675674

@@ -696,15 +695,16 @@ struct BasicBlock : private LIR::Range
696695
{
697696
assert(KindIs(BBJ_COND));
698697
assert(bbFalseTarget != nullptr);
699-
assert(target != nullptr);
700698
return (bbFalseTarget == target);
701699
}
702700

703701
void SetCond(BasicBlock* trueTarget, BasicBlock* falseTarget)
704702
{
703+
// Switch lowering may temporarily set a block to a BBJ_COND
704+
// with a null false target if it is the last block in the list.
705+
// This invalid state is eventually fixed, so allow it in the below assert.
706+
assert((falseTarget != nullptr) || (falseTarget == bbNext));
705707
assert(trueTarget != nullptr);
706-
// TODO-NoFallThrough: Allow falseTarget to diverge from bbNext
707-
assert(falseTarget == bbNext);
708708
bbKind = BBJ_COND;
709709
bbTrueTarget = trueTarget;
710710
bbFalseTarget = falseTarget;

‎src/coreclr/jit/codegenarm.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,13 @@ void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
13171317
regNumber reg = genConsumeReg(op);
13181318
inst_RV_RV(INS_tst, reg, reg, genActualType(op));
13191319
inst_JMP(EJ_ne, compiler->compCurBB->GetTrueTarget());
1320+
1321+
// If we cannot fall into the false target, emit a jump to it
1322+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
1323+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
1324+
{
1325+
inst_JMP(EJ_jmp, falseTarget);
1326+
}
13201327
}
13211328

13221329
//------------------------------------------------------------------------

‎src/coreclr/jit/codegenarm64.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4650,6 +4650,13 @@ void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
46504650
GenTree* op = jtrue->gtGetOp1();
46514651
regNumber reg = genConsumeReg(op);
46524652
GetEmitter()->emitIns_J_R(INS_cbnz, emitActualTypeSize(op), compiler->compCurBB->GetTrueTarget(), reg);
4653+
4654+
// If we cannot fall into the false target, emit a jump to it
4655+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
4656+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
4657+
{
4658+
inst_JMP(EJ_jmp, falseTarget);
4659+
}
46534660
}
46544661

46554662
//------------------------------------------------------------------------
@@ -4877,6 +4884,13 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree)
48774884

48784885
GetEmitter()->emitIns_J_R(ins, attr, compiler->compCurBB->GetTrueTarget(), reg);
48794886
}
4887+
4888+
// If we cannot fall into the false target, emit a jump to it
4889+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
4890+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
4891+
{
4892+
inst_JMP(EJ_jmp, falseTarget);
4893+
}
48804894
}
48814895

48824896
//---------------------------------------------------------------------

‎src/coreclr/jit/codegencommon.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ void CodeGen::genMarkLabelsForCodegen()
397397
block->GetTrueTarget()->SetFlags(BBF_HAS_LABEL);
398398

399399
// If we need a jump to the false target, give it a label
400-
if (!block->CanRemoveJumpToFalseTarget(compiler))
400+
if (!block->CanRemoveJumpToTarget(block->GetFalseTarget(), compiler))
401401
{
402402
JITDUMP(" " FMT_BB " : branch target\n", block->GetFalseTarget()->bbNum);
403403
block->GetFalseTarget()->SetFlags(BBF_HAS_LABEL);

‎src/coreclr/jit/codegenlinear.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2621,9 +2621,10 @@ void CodeGen::genCodeForJcc(GenTreeCC* jcc)
26212621
inst_JCC(jcc->gtCondition, compiler->compCurBB->GetTrueTarget());
26222622

26232623
// If we cannot fall into the false target, emit a jump to it
2624-
if (!compiler->compCurBB->CanRemoveJumpToFalseTarget(compiler))
2624+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
2625+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
26252626
{
2626-
inst_JMP(EJ_jmp, compiler->compCurBB->GetFalseTarget());
2627+
inst_JMP(EJ_jmp, falseTarget);
26272628
}
26282629
}
26292630

‎src/coreclr/jit/codegenloongarch64.cpp‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4330,6 +4330,13 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree)
43304330
assert(regs != 0);
43314331

43324332
emit->emitIns_J(ins, compiler->compCurBB->GetTrueTarget(), regs); // 5-bits;
4333+
4334+
// If we cannot fall into the false target, emit a jump to it
4335+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
4336+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
4337+
{
4338+
inst_JMP(EJ_jmp, falseTarget);
4339+
}
43334340
}
43344341

43354342
//---------------------------------------------------------------------
@@ -4874,16 +4881,31 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
48744881

48754882
case GT_JCC:
48764883
{
4884+
#if !FEATURE_FIXED_OUT_ARGS
48774885
BasicBlock* tgtBlock = compiler->compCurBB->KindIs(BBJ_COND) ? compiler->compCurBB->GetTrueTarget()
48784886
: compiler->compCurBB->GetTarget();
4879-
#if !FEATURE_FIXED_OUT_ARGS
48804887
assert((tgtBlock->bbTgtStkDepth * sizeof(int) == genStackLevel) || isFramePointerUsed());
48814888
#endif // !FEATURE_FIXED_OUT_ARGS
48824889

48834890
GenTreeCC* jcc = treeNode->AsCC();
48844891
assert(jcc->gtCondition.Is(GenCondition::EQ, GenCondition::NE));
48854892
instruction ins = jcc->gtCondition.Is(GenCondition::EQ) ? INS_bceqz : INS_bcnez;
4886-
emit->emitIns_J(ins, tgtBlock, (int)1 /* cc */);
4893+
4894+
if (compiler->compCurBB->KindIs(BBJ_COND))
4895+
{
4896+
emit->emitIns_J(ins, compiler->compCurBB->GetTrueTarget(), (int)1 /* cc */);
4897+
4898+
// If we cannot fall into the false target, emit a jump to it
4899+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
4900+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
4901+
{
4902+
inst_JMP(EJ_jmp, falseTarget);
4903+
}
4904+
}
4905+
else
4906+
{
4907+
emit->emitIns_J(ins, compiler->compCurBB->GetTarget(), (int)1 /* cc */);
4908+
}
48874909
}
48884910
break;
48894911

‎src/coreclr/jit/codegenriscv64.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3920,6 +3920,13 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree)
39203920
assert(regs != 0);
39213921

39223922
emit->emitIns_J(ins, compiler->compCurBB->GetTrueTarget(), regs); // 5-bits;
3923+
3924+
// If we cannot fall into the false target, emit a jump to it
3925+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
3926+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
3927+
{
3928+
inst_JMP(EJ_jmp, falseTarget);
3929+
}
39233930
}
39243931

39253932
//---------------------------------------------------------------------

‎src/coreclr/jit/codegenxarch.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,6 +1453,13 @@ void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
14531453
regNumber reg = genConsumeReg(op);
14541454
inst_RV_RV(INS_test, reg, reg, genActualType(op));
14551455
inst_JMP(EJ_jne, compiler->compCurBB->GetTrueTarget());
1456+
1457+
// If we cannot fall into the false target, emit a jump to it
1458+
BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget();
1459+
if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler))
1460+
{
1461+
inst_JMP(EJ_jmp, falseTarget);
1462+
}
14561463
}
14571464

14581465
//------------------------------------------------------------------------

‎src/coreclr/jit/compiler.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5956,7 +5956,7 @@ class Compiler
59565956

59575957
void fgUpdateLoopsAfterCompacting(BasicBlock* block, BasicBlock* bNext);
59585958

5959-
BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst);
5959+
BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst, bool noFallThroughQuirk = false);
59605960

59615961
bool fgRenumberBlocks();
59625962

@@ -6003,8 +6003,6 @@ class Compiler
60036003

60046004
bool fgOptimizeSwitchBranches(BasicBlock* block);
60056005

6006-
bool fgOptimizeBranchToNext(BasicBlock* block, BasicBlock* bNext, BasicBlock* bPrev);
6007-
60086006
bool fgOptimizeSwitchJumps();
60096007
#ifdef DEBUG
60106008
void fgPrintEdgeWeights();

0 commit comments

Comments
 (0)