Skip to content

Commit 369be31

Browse files
authored
[X86,SimplifyCFG] Support conditional faulting load or store only (#132032)
This is to fix a bug when a target only support conditional faulting load, see test case hoist_store_without_cstore. Split `-simplifycfg-hoist-loads-stores-with-cond-faulting` into `-simplifycfg-hoist-loads-with-cond-faulting` and `-simplifycfg-hoist-stores-with-cond-faulting` to control conditional faulting load and store respectively.
1 parent 6b59b33 commit 369be31

File tree

2 files changed

+627
-184
lines changed

2 files changed

+627
-184
lines changed

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+28-18
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,13 @@ static cl::opt<bool>
120120
HoistCommon("simplifycfg-hoist-common", cl::Hidden, cl::init(true),
121121
cl::desc("Hoist common instructions up to the parent block"));
122122

123-
static cl::opt<bool> HoistLoadsStoresWithCondFaulting(
124-
"simplifycfg-hoist-loads-stores-with-cond-faulting", cl::Hidden,
125-
cl::init(true),
126-
cl::desc("Hoist loads/stores if the target supports "
127-
"conditional faulting"));
123+
static cl::opt<bool> HoistLoadsWithCondFaulting(
124+
"simplifycfg-hoist-loads-with-cond-faulting", cl::Hidden, cl::init(true),
125+
cl::desc("Hoist loads if the target supports conditional faulting"));
126+
127+
static cl::opt<bool> HoistStoresWithCondFaulting(
128+
"simplifycfg-hoist-stores-with-cond-faulting", cl::Hidden, cl::init(true),
129+
cl::desc("Hoist stores if the target supports conditional faulting"));
128130

129131
static cl::opt<unsigned> HoistLoadsStoresWithCondFaultingThreshold(
130132
"hoist-loads-stores-with-cond-faulting-threshold", cl::Hidden, cl::init(6),
@@ -1682,22 +1684,22 @@ static bool areIdenticalUpToCommutativity(const Instruction *I1,
16821684
static void hoistConditionalLoadsStores(
16831685
BranchInst *BI,
16841686
SmallVectorImpl<Instruction *> &SpeculatedConditionalLoadsStores,
1685-
std::optional<bool> Invert) {
1687+
std::optional<bool> Invert, Instruction *Sel) {
16861688
auto &Context = BI->getParent()->getContext();
16871689
auto *VCondTy = FixedVectorType::get(Type::getInt1Ty(Context), 1);
16881690
auto *Cond = BI->getOperand(0);
16891691
// Construct the condition if needed.
16901692
BasicBlock *BB = BI->getParent();
1691-
IRBuilder<> Builder(
1692-
Invert.has_value() ? SpeculatedConditionalLoadsStores.back() : BI);
16931693
Value *Mask = nullptr;
16941694
Value *MaskFalse = nullptr;
16951695
Value *MaskTrue = nullptr;
16961696
if (Invert.has_value()) {
1697+
IRBuilder<> Builder(Sel ? Sel : SpeculatedConditionalLoadsStores.back());
16971698
Mask = Builder.CreateBitCast(
16981699
*Invert ? Builder.CreateXor(Cond, ConstantInt::getTrue(Context)) : Cond,
16991700
VCondTy);
17001701
} else {
1702+
IRBuilder<> Builder(BI);
17011703
MaskFalse = Builder.CreateBitCast(
17021704
Builder.CreateXor(Cond, ConstantInt::getTrue(Context)), VCondTy);
17031705
MaskTrue = Builder.CreateBitCast(Cond, VCondTy);
@@ -1723,13 +1725,20 @@ static void hoistConditionalLoadsStores(
17231725
PHINode *PN = nullptr;
17241726
Value *PassThru = nullptr;
17251727
if (Invert.has_value())
1726-
for (User *U : I->users())
1728+
for (User *U : I->users()) {
17271729
if ((PN = dyn_cast<PHINode>(U))) {
17281730
PassThru = Builder.CreateBitCast(
17291731
PeekThroughBitcasts(PN->getIncomingValueForBlock(BB)),
17301732
FixedVectorType::get(Ty, 1));
1731-
break;
1733+
} else if (auto *Ins = cast<Instruction>(U);
1734+
Sel && Ins->getParent() == BB) {
1735+
// This happens when store or/and a speculative instruction between
1736+
// load and store were hoisted to the BB. Make sure the masked load
1737+
// inserted before its use.
1738+
// We assume there's one of such use.
1739+
Builder.SetInsertPoint(Ins);
17321740
}
1741+
}
17331742
MaskedLoadStore = Builder.CreateMaskedLoad(
17341743
FixedVectorType::get(Ty, 1), Op0, LI->getAlign(), Mask, PassThru);
17351744
Value *NewLoadStore = Builder.CreateBitCast(MaskedLoadStore, Ty);
@@ -1770,10 +1779,10 @@ static bool isSafeCheapLoadStore(const Instruction *I,
17701779
// Not handle volatile or atomic.
17711780
bool IsStore = false;
17721781
if (auto *L = dyn_cast<LoadInst>(I)) {
1773-
if (!L->isSimple())
1782+
if (!L->isSimple() || !HoistLoadsWithCondFaulting)
17741783
return false;
17751784
} else if (auto *S = dyn_cast<StoreInst>(I)) {
1776-
if (!S->isSimple())
1785+
if (!S->isSimple() || !HoistStoresWithCondFaulting)
17771786
return false;
17781787
IsStore = true;
17791788
} else
@@ -3214,8 +3223,7 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
32143223
SmallVector<Instruction *, 4> SpeculatedDbgIntrinsics;
32153224

32163225
unsigned SpeculatedInstructions = 0;
3217-
bool HoistLoadsStores = HoistLoadsStoresWithCondFaulting &&
3218-
Options.HoistLoadsStoresWithCondFaulting;
3226+
bool HoistLoadsStores = Options.HoistLoadsStoresWithCondFaulting;
32193227
SmallVector<Instruction *, 2> SpeculatedConditionalLoadsStores;
32203228
Value *SpeculatedStoreValue = nullptr;
32213229
StoreInst *SpeculatedStore = nullptr;
@@ -3310,6 +3318,7 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
33103318
// If we get here, we can hoist the instruction and if-convert.
33113319
LLVM_DEBUG(dbgs() << "SPECULATIVELY EXECUTING BB" << *ThenBB << "\n";);
33123320

3321+
Instruction *Sel = nullptr;
33133322
// Insert a select of the value of the speculated store.
33143323
if (SpeculatedStoreValue) {
33153324
IRBuilder<NoFolder> Builder(BI);
@@ -3320,6 +3329,7 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
33203329
std::swap(TrueV, FalseV);
33213330
Value *S = Builder.CreateSelect(
33223331
BrCond, TrueV, FalseV, "spec.store.select", BI);
3332+
Sel = cast<Instruction>(S);
33233333
SpeculatedStore->setOperand(0, S);
33243334
SpeculatedStore->applyMergedLocation(BI->getDebugLoc(),
33253335
SpeculatedStore->getDebugLoc());
@@ -3392,7 +3402,8 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(BranchInst *BI,
33923402
std::prev(ThenBB->end()));
33933403

33943404
if (!SpeculatedConditionalLoadsStores.empty())
3395-
hoistConditionalLoadsStores(BI, SpeculatedConditionalLoadsStores, Invert);
3405+
hoistConditionalLoadsStores(BI, SpeculatedConditionalLoadsStores, Invert,
3406+
Sel);
33963407

33973408
// Insert selects and rewrite the PHI operands.
33983409
IRBuilder<NoFolder> Builder(BI);
@@ -8020,8 +8031,7 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
80208031
hoistCommonCodeFromSuccessors(BI, !Options.HoistCommonInsts))
80218032
return requestResimplify();
80228033

8023-
if (BI && HoistLoadsStoresWithCondFaulting &&
8024-
Options.HoistLoadsStoresWithCondFaulting &&
8034+
if (BI && Options.HoistLoadsStoresWithCondFaulting &&
80258035
isProfitableToSpeculate(BI, std::nullopt, TTI)) {
80268036
SmallVector<Instruction *, 2> SpeculatedConditionalLoadsStores;
80278037
auto CanSpeculateConditionalLoadsStores = [&]() {
@@ -8044,7 +8054,7 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
80448054

80458055
if (CanSpeculateConditionalLoadsStores()) {
80468056
hoistConditionalLoadsStores(BI, SpeculatedConditionalLoadsStores,
8047-
std::nullopt);
8057+
std::nullopt, nullptr);
80488058
return requestResimplify();
80498059
}
80508060
}

0 commit comments

Comments
 (0)