[IR] Don't allow successors() over block without terminators#186646
Open
[IR] Don't allow successors() over block without terminators#186646
Conversation
llvmbot
added
llvm:ir
llvm:analysis
Member
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-llvm-ir Author: Alexis Engelke (aengelke) ChangesThere's no point constructing a dominator tree or similar on known-broken IR. Generally, functions should be able to assume that IR is valid (i.e., passes the verifier). Users of this "feature" were:
Full diff: https://github.com/llvm/llvm-project/pull/186646.diff 9 Files Affected:
diff --git a/llvm/include/llvm/IR/CFG.h b/llvm/include/llvm/IR/CFG.h
index c0d2218c24322..e8158fdc243b1 100644
--- a/llvm/include/llvm/IR/CFG.h
+++ b/llvm/include/llvm/IR/CFG.h
@@ -141,16 +141,16 @@ using succ_range = iterator_range<succ_iterator>;
using const_succ_range = iterator_range<const_succ_iterator>;
inline succ_iterator succ_begin(Instruction *I) {
- return I ? I->successors().begin() : succ_iterator(nullptr);
+ return I->successors().begin();
}
inline const_succ_iterator succ_begin(const Instruction *I) {
- return I ? I->successors().begin() : const_succ_iterator(nullptr);
+ return I->successors().begin();
}
inline succ_iterator succ_end(Instruction *I) {
- return I ? I->successors().end() : succ_iterator(nullptr);
+ return I->successors().end();
}
inline const_succ_iterator succ_end(const Instruction *I) {
- return I ? I->successors().end() : const_succ_iterator(nullptr);
+ return I->successors().end();
}
inline bool succ_empty(const Instruction *I) {
return succ_begin(I) == succ_end(I);
@@ -159,10 +159,10 @@ inline unsigned succ_size(const Instruction *I) {
return std::distance(succ_begin(I), succ_end(I));
}
inline succ_range successors(Instruction *I) {
- return succ_range(succ_begin(I), succ_end(I));
+ return I->successors();
}
inline const_succ_range successors(const Instruction *I) {
- return const_succ_range(succ_begin(I), succ_end(I));
+ return I->successors();
}
inline succ_iterator succ_begin(BasicBlock *BB) {
@@ -184,10 +184,10 @@ inline unsigned succ_size(const BasicBlock *BB) {
return std::distance(succ_begin(BB), succ_end(BB));
}
inline succ_range successors(BasicBlock *BB) {
- return succ_range(succ_begin(BB), succ_end(BB));
+ return successors(BB->getTerminator());
}
inline const_succ_range successors(const BasicBlock *BB) {
- return const_succ_range(succ_begin(BB), succ_end(BB));
+ return successors(BB->getTerminator());
}
//===--------------------------------------------------------------------===//
diff --git a/llvm/lib/FuzzMutate/RandomIRBuilder.cpp b/llvm/lib/FuzzMutate/RandomIRBuilder.cpp
index 4e4732ae08c1b..b5946b98bb12b 100644
--- a/llvm/lib/FuzzMutate/RandomIRBuilder.cpp
+++ b/llvm/lib/FuzzMutate/RandomIRBuilder.cpp
@@ -21,11 +21,23 @@
using namespace llvm;
using namespace fuzzerop;
+static DominatorTree getDomTree(Function &F) {
+ // Dominator tree construction requires that all blocks have terminators.
+ SmallVector<Instruction *> AddedInsts;
+ for (BasicBlock &BB : F)
+ if (!BB.getTerminator())
+ AddedInsts.push_back(new UnreachableInst(F.getContext(), &BB));
+ DominatorTree DT(F);
+ for (Instruction *I : AddedInsts)
+ I->eraseFromParent();
+ return DT;
+}
+
/// Return a vector of Blocks that dominates this block, excluding current
/// block.
static std::vector<BasicBlock *> getDominators(BasicBlock *BB) {
std::vector<BasicBlock *> ret;
- DominatorTree DT(*BB->getParent());
+ DominatorTree DT = getDomTree(*BB->getParent());
DomTreeNode *Node = DT.getNode(BB);
// It's possible that an orphan block is not in the dom tree. In that case we
// just return nothing.
@@ -43,7 +55,7 @@ static std::vector<BasicBlock *> getDominators(BasicBlock *BB) {
/// Return a vector of Blocks that is dominated by this block, excluding current
/// block
static std::vector<BasicBlock *> getDominatees(BasicBlock *BB) {
- DominatorTree DT(*BB->getParent());
+ DominatorTree DT = getDomTree(*BB->getParent());
std::vector<BasicBlock *> ret;
DomTreeNode *Parent = DT.getNode(BB);
// It's possible that an orphan block is not in the dom tree. In that case we
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3cdc75ca9869e..593595f0a831e 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -410,10 +410,9 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
// pass manager to provide this as it isolates us from a potentially
// out-of-date dominator tree and makes it significantly more complex to run
// this code outside of a pass manager.
- // FIXME: It's really gross that we have to cast away constness here.
- if (!F.empty())
- DT.recalculate(const_cast<Function &>(F));
+ // First check that every basic block has a terminator, otherwise we can't
+ // even inspect the CFG.
for (const BasicBlock &BB : F) {
if (!BB.empty() && BB.back().isTerminator())
continue;
@@ -427,6 +426,10 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
return false;
}
+ // FIXME: It's really gross that we have to cast away constness here.
+ if (!F.empty())
+ DT.recalculate(const_cast<Function &>(F));
+
auto FailureCB = [this](const Twine &Message) {
this->CheckFailed(Message);
};
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index f8e0ad2b7b661..17aa7cc185b66 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -346,6 +346,8 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
if (PredecessorWithTwoSuccessors) {
// Delete the unconditional branch from BB.
BB->back().eraseFromParent();
+ // Add unreachable to now empty BB.
+ new UnreachableInst(BB->getContext(), BB);
// Update branch in the predecessor.
PredBB_BI->setSuccessor(FallThruPath, NewSucc);
@@ -355,6 +357,8 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
// Move terminator instruction.
BB->back().moveBeforePreserving(*PredBB, PredBB->end());
+ // Add unreachable to now empty BB.
+ new UnreachableInst(BB->getContext(), BB);
// Terminator may be a memory accessing instruction too.
if (MSSAU)
@@ -362,8 +366,6 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
MSSAU->getMemorySSA()->getMemoryAccess(PredBB->getTerminator())))
MSSAU->moveToPlace(MUD, PredBB, MemorySSA::End);
}
- // Add unreachable to now empty BB.
- new UnreachableInst(BB->getContext(), BB);
// Inherit predecessors name if it exists.
if (!PredBB->hasName())
diff --git a/llvm/test/Transforms/LoopSimplifyCFG/mssa_term.ll b/llvm/test/Transforms/LoopSimplifyCFG/mssa_term.ll
new file mode 100644
index 0000000000000..a2936727ea6ad
--- /dev/null
+++ b/llvm/test/Transforms/LoopSimplifyCFG/mssa_term.ll
@@ -0,0 +1,46 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -S -passes="loop-mssa(loop-simplifycfg,simple-loop-unswitch)" < %s | FileCheck %s
+
+; Check that IR is valid when MemorySSA is updated during MergeBlockIntoPredecessor.
+
+define i32 @f1(i1 %cond) personality ptr null {
+; CHECK-LABEL: define i32 @f1(
+; CHECK-SAME: i1 [[COND:%.*]]) personality ptr null {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br i1 [[COND]], label %[[ENTRY_SPLIT:.*]], label %[[COMMON_RET_LOOPEXIT:.*]]
+; CHECK: [[ENTRY_SPLIT]]:
+; CHECK-NEXT: br label %[[FOR_COND:.*]]
+; CHECK: [[FOR_COND]]:
+; CHECK-NEXT: [[CALL26:%.*]] = invoke i32 @f2(ptr null, ptr null, ptr null)
+; CHECK-NEXT: to label %[[FOR_COND]] unwind label %[[LPAD24:.*]]
+; CHECK: [[COMMON_RET_LOOPEXIT]]:
+; CHECK-NEXT: br label %[[COMMON_RET:.*]]
+; CHECK: [[COMMON_RET]]:
+; CHECK-NEXT: [[COMMON_RET_OP:%.*]] = phi i32 [ 0, %[[LPAD24]] ], [ 0, %[[COMMON_RET_LOOPEXIT]] ]
+; CHECK-NEXT: ret i32 [[COMMON_RET_OP]]
+; CHECK: [[LPAD24]]:
+; CHECK-NEXT: [[LPAD:%.*]] = landingpad { ptr, i32 }
+; CHECK-NEXT: cleanup
+; CHECK-NEXT: br label %[[COMMON_RET]]
+;
+entry:
+ br label %for.cond
+
+for.cond: ; preds = %if.end19, %entry
+ br i1 %cond, label %if.end19, label %common.ret
+
+common.ret: ; preds = %lpad24, %for.cond
+ %common.ret.op = phi i32 [ 0, %lpad24 ], [ 0, %for.cond ]
+ ret i32 %common.ret.op
+
+if.end19: ; preds = %for.cond
+ %call26 = invoke i32 @f2(ptr null, ptr null, ptr null)
+ to label %for.cond unwind label %lpad24
+
+lpad24: ; preds = %if.end19
+ %lpad = landingpad { ptr, i32 }
+ cleanup
+ br label %common.ret
+}
+
+declare i32 @f2(ptr, ptr, ptr)
diff --git a/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp b/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
index bae1f1c508af3..0541f257b20d2 100644
--- a/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
+++ b/llvm/unittests/Analysis/BasicAliasAnalysisTest.cpp
@@ -79,6 +79,7 @@ TEST_F(BasicAATest, AliasInstWithObjectOfImpreciseSize) {
BasicBlock *Entry(BasicBlock::Create(C, "", F));
B.SetInsertPoint(Entry);
+ B.CreateRetVoid();
Value *IncomingI32Ptr = F->arg_begin();
@@ -119,6 +120,7 @@ TEST_F(BasicAATest, AliasInstWithFullObjectOfImpreciseSize) {
AllocaInst *I8 = B.CreateAlloca(B.getInt8Ty(), B.getInt32(2));
auto *I8AtUncertainOffset =
cast<GetElementPtrInst>(B.CreatePtrAdd(I8, ArbitraryI32));
+ B.CreateRetVoid();
auto &AllAnalyses = setupAnalyses();
BasicAAResult &BasicAA = AllAnalyses.BAA;
diff --git a/llvm/unittests/Analysis/DomTreeUpdaterTest.cpp b/llvm/unittests/Analysis/DomTreeUpdaterTest.cpp
index 1a8160f14a0a8..4fe3e701d9f83 100644
--- a/llvm/unittests/Analysis/DomTreeUpdaterTest.cpp
+++ b/llvm/unittests/Analysis/DomTreeUpdaterTest.cpp
@@ -776,6 +776,7 @@ TEST(DomTreeUpdater, LazyUpdateDeduplicationTest) {
// CFG Change: remove bb0 -> bb1.
EXPECT_EQ(BB0->getTerminator()->getNumSuccessors(), 1u);
BB0->getTerminator()->eraseFromParent();
+ new UnreachableInst(Context, BB0);
// Update the DTU and simulate invalid updates.
DTU.applyUpdatesPermissive({{DominatorTree::Delete, BB0, BB1},
diff --git a/llvm/unittests/Analysis/MemorySSATest.cpp b/llvm/unittests/Analysis/MemorySSATest.cpp
index a2e4f99a07d22..38c2e41776ed8 100644
--- a/llvm/unittests/Analysis/MemorySSATest.cpp
+++ b/llvm/unittests/Analysis/MemorySSATest.cpp
@@ -91,6 +91,7 @@ TEST_F(MemorySSATest, CreateALoad) {
B.CreateStore(B.getInt8(16), PointerArg);
UncondBrInst::Create(Merge, Left);
UncondBrInst::Create(Merge, Right);
+ ReturnInst::Create(C, Merge);
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -129,6 +130,7 @@ TEST_F(MemorySSATest, CreateLoadsAndStoreUpdater) {
B.CreateBr(Merge);
B.SetInsertPoint(Right);
B.CreateBr(Merge);
+ ReturnInst::Create(C, Merge);
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -218,6 +220,7 @@ TEST_F(MemorySSATest, CreateALoadUpdater) {
B.CreateBr(Merge);
B.SetInsertPoint(Right);
B.CreateBr(Merge);
+ ReturnInst::Create(C, Merge);
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -261,6 +264,7 @@ TEST_F(MemorySSATest, SinkLoad) {
B.CreateBr(Merge);
B.SetInsertPoint(Right);
B.CreateBr(Merge);
+ ReturnInst::Create(C, Merge);
// Load in left block
B.SetInsertPoint(Left, Left->begin());
@@ -312,6 +316,8 @@ TEST_F(MemorySSATest, MoveAStore) {
UncondBrInst::Create(Merge, Right);
B.SetInsertPoint(Merge);
B.CreateLoad(B.getInt8Ty(), PointerArg);
+ B.CreateRetVoid();
+
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
MemorySSAUpdater Updater(&MSSA);
@@ -347,6 +353,8 @@ TEST_F(MemorySSATest, MoveAStoreUpdater) {
UncondBrInst::Create(Merge, Right);
B.SetInsertPoint(Merge);
auto *MergeLoad = B.CreateLoad(B.getInt8Ty(), PointerArg);
+ B.CreateRetVoid();
+
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
MemorySSAUpdater Updater(&MSSA);
@@ -392,6 +400,8 @@ TEST_F(MemorySSATest, MoveAStoreUpdaterMove) {
UncondBrInst::Create(Merge, Right);
B.SetInsertPoint(Merge);
auto *MergeLoad = B.CreateLoad(B.getInt8Ty(), PointerArg);
+ B.CreateRetVoid();
+
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
MemorySSAUpdater Updater(&MSSA);
@@ -435,6 +445,8 @@ TEST_F(MemorySSATest, MoveAStoreAllAround) {
UncondBrInst::Create(Merge, Right);
B.SetInsertPoint(Merge);
auto *MergeLoad = B.CreateLoad(B.getInt8Ty(), PointerArg);
+ B.CreateRetVoid();
+
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
MemorySSAUpdater Updater(&MSSA);
@@ -488,6 +500,7 @@ TEST_F(MemorySSATest, RemoveAPhi) {
UncondBrInst::Create(Merge, Right);
B.SetInsertPoint(Merge);
LoadInst *LoadInst = B.CreateLoad(B.getInt8Ty(), PointerArg);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -532,6 +545,7 @@ TEST_F(MemorySSATest, RemoveMemoryAccess) {
UncondBrInst::Create(Merge, Right);
B.SetInsertPoint(Merge);
LoadInst *LoadInst = B.CreateLoad(B.getInt8Ty(), PointerArg);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -598,6 +612,7 @@ TEST_F(MemorySSATest, TestTripleStore) {
StoreInst *S1 = B.CreateStore(ConstantInt::get(Int8, 0), Alloca);
StoreInst *S2 = B.CreateStore(ConstantInt::get(Int8, 1), Alloca);
StoreInst *S3 = B.CreateStore(ConstantInt::get(Int8, 2), Alloca);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -628,6 +643,7 @@ TEST_F(MemorySSATest, TestStoreAndLoad) {
Value *Alloca = B.CreateAlloca(Int8, ConstantInt::get(Int8, 1), "A");
Instruction *SI = B.CreateStore(ConstantInt::get(Int8, 0), Alloca);
Instruction *LI = B.CreateLoad(Int8, Alloca);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -657,6 +673,7 @@ TEST_F(MemorySSATest, TestStoreDoubleQuery) {
Type *Int8 = Type::getInt8Ty(C);
Value *Alloca = B.CreateAlloca(Int8, ConstantInt::get(Int8, 1), "A");
StoreInst *SI = B.CreateStore(ConstantInt::get(Int8, 0), Alloca);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -721,6 +738,7 @@ TEST_F(MemorySSATest, PartialWalkerCacheWithPhis) {
B.SetInsertPoint(IfEnd);
Instruction *BelowPhi = B.CreateStore(Zero, AllocA);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -768,6 +786,7 @@ TEST_F(MemorySSATest, WalkerInvariantLoadOpt) {
Instruction *Store = B.CreateStore(One, AllocA);
Instruction *Load = B.CreateLoad(Int8, AllocA);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -797,6 +816,7 @@ TEST_F(MemorySSATest, WalkerReopt) {
Value *AllocaB = B.CreateAlloca(Int8, ConstantInt::get(Int8, 1), "B");
Instruction *SIB = B.CreateStore(ConstantInt::get(Int8, 0), AllocaB);
Instruction *LIA = B.CreateLoad(Int8, AllocaA);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -835,6 +855,7 @@ TEST_F(MemorySSATest, MoveAboveMemoryDef) {
StoreInst *StoreC = B.CreateStore(ConstantInt::get(Int8, 4), C);
StoreInst *StoreA2 = B.CreateStore(ConstantInt::get(Int8, 4), A);
LoadInst *LoadC = B.CreateLoad(Int8, C);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -891,6 +912,7 @@ TEST_F(MemorySSATest, Irreducible) {
B.SetInsertPoint(LoopMainBB);
B.CreateCondBr(B.getTrue(), LoopStartBB, AfterLoopBB);
B.SetInsertPoint(AfterLoopBB);
+ B.CreateRetVoid();
Argument *FirstArg = &*F->arg_begin();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -923,6 +945,7 @@ TEST_F(MemorySSATest, MoveToBeforeLiveOnEntryInvalidatesCache) {
Value *A = B.CreateAlloca(B.getInt8Ty());
StoreInst *StoreA = B.CreateStore(B.getInt8(0), A);
StoreInst *StoreB = B.CreateStore(B.getInt8(0), A);
+ B.CreateRetVoid();
setupAnalyses();
@@ -969,6 +992,7 @@ TEST_F(MemorySSATest, RemovingDefInvalidatesCache) {
StoreInst *StoreX1 = B.CreateStore(B.getInt8(0), X);
StoreInst *StoreY = B.CreateStore(B.getInt8(0), Y);
StoreInst *StoreX2 = B.CreateStore(B.getInt8(0), X);
+ B.CreateRetVoid();
setupAnalyses();
@@ -1005,6 +1029,7 @@ TEST_F(MemorySSATest, TestStoreMustAlias) {
StoreInst *SB2 = B.CreateStore(ConstantInt::get(Int8, 2), AllocaB);
StoreInst *SA3 = B.CreateStore(ConstantInt::get(Int8, 3), AllocaA);
StoreInst *SB3 = B.CreateStore(ConstantInt::get(Int8, 3), AllocaB);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1056,6 +1081,7 @@ TEST_F(MemorySSATest, TestStoreMayAlias) {
StoreInst *SC2 = B.CreateStore(ConstantInt::get(Int8, 5), AllocaC);
// Store into arg2, must alias store to arg2 => must
StoreInst *SB3 = B.CreateStore(ConstantInt::get(Int8, 6), PointerB);
+ B.CreateRetVoid();
std::initializer_list<StoreInst *> Sts = {SA1, SB1, SC1, SA2, SB2, SC2, SB3};
setupAnalyses();
@@ -1126,6 +1152,7 @@ TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
Instruction *FooStore = B.CreateStore(B.getInt8(0), Foo);
Instruction *BarStore = B.CreateStore(B.getInt8(0), Bar);
Instruction *BazMemSet = B.CreateMemSet(Baz, B.getInt8(0), 1, Align(1));
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1236,6 +1263,7 @@ TEST_F(MemorySSATest, TestOptimizedDefsAreProperUses) {
StoreInst *StoreA = B.CreateStore(ConstantInt::get(Int8, 0), AllocA);
StoreInst *StoreB = B.CreateStore(ConstantInt::get(Int8, 1), AllocB);
StoreInst *StoreA2 = B.CreateStore(ConstantInt::get(Int8, 2), AllocA);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1319,6 +1347,7 @@ TEST_F(MemorySSATest, TestAddedEdgeToBlockWithPhiNotOpt) {
UncondBrInst::Create(Exit, Body);
B.SetInsertPoint(Exit);
StoreInst *S1 = B.CreateStore(B.getInt8(16), PointerArg);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1380,6 +1409,7 @@ TEST_F(MemorySSATest, TestAddedEdgeToBlockWithPhiOpt) {
B.SetInsertPoint(Exit);
StoreInst *S2 = B.CreateStore(B.getInt8(16), PointerArg);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1452,6 +1482,7 @@ TEST_F(MemorySSATest, TestAddedEdgeToBlockWithNoPhiAddNewPhis) {
B.SetInsertPoint(FBlock);
B.CreateStore(B.getInt8(16), PointerArg);
UncondBrInst::Create(EBlock, FBlock);
+ ReturnInst::Create(C, EBlock);
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1486,6 +1517,7 @@ TEST_F(MemorySSATest, TestCallClobber) {
Instruction *StorePointer1 = B.CreateStore(B.getInt8(0), Pointer1);
Instruction *StorePointer2 = B.CreateStore(B.getInt8(0), Pointer2);
Instruction *MemSet = B.CreateMemSet(Pointer2, B.getInt8(0), 1, Align(1));
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1519,6 +1551,7 @@ TEST_F(MemorySSATest, TestLoadClobber) {
B.CreateLoad(B.getInt8Ty(), Pointer1, /* Volatile */ true);
Instruction *LoadPointer2 =
B.CreateLoad(B.getInt8Ty(), Pointer2, /* Volatile */ true);
+ B.CreateRetVoid();
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
diff --git a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
index 503d3a0f9bfa9..0c7720e9c4f1f 100644
--- a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
+++ b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
@@ -44,6 +44,7 @@ TEST(SSAUpdaterBulk, SimpleMerge) {
// %6 = add i32 %3, 6
// %7 = add i32 %2, %4
// %8 = sub i32 %2, %4
+ // ret void
Argument *FirstArg = &*(F->arg_begin());
BasicBlock *IfBB = BasicBlock::Create(C, "if", F);
BasicBlock *TrueBB = BasicBlock::Create(C, "true", F);
@@ -68,6 +69,7 @@ TEST(SSAUpdaterBulk, SimpleMerge) {
auto *I2 = cast<Instruction>(B.CreateAdd(AddOp2, ConstantInt::get(I32Ty, 6)));
auto *I3 = cast<Instruction>(B.CreateAdd(SubOp1, SubOp2));
auto *I4 = cast<Instruction>(B.CreateSub(SubOp1, SubOp2));
+ B.CreateRetVoid();
// Now rewrite uses in instructions %5, %6, %7. They need to use a phi, which
// SSAUpdater should insert into %merge.
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
|
Looks like there are some Clang/MLIR test failures related to OpenMP. |
Contributor
Author
|
sigh. I know ~nothing how this OpenMP code works and don't care enough to fix it at this point... Polly is also affected. |
Contributor
|
Something we could do is to move the null check to Though I think we should land the compatibility fixes in any case. |
Contributor
Author
|
So the OpenMP IR builder does sketchy things also on other occasions and works around limitations by temporarily adding unreachable instructions. Can't hurt to do this three more times... same for Polly. The c-t impact is 0.10% stage2-O3 -- more than I expected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There's no point constructing a dominator tree or similar on known-broken IR. Generally, functions should be able to assume that IR is valid (i.e., passes the verifier). Users of this "feature" were: