From e200e9264d513d8ea967fd07fdc49593932e5fc3 Mon Sep 17 00:00:00 2001 From: Alexis Engelke Date: Sat, 14 Mar 2026 14:53:24 +0000 Subject: [PATCH 1/4] [IR] Don't allow successors() over block without terminators --- llvm/include/llvm/IR/CFG.h | 16 +++---- llvm/lib/FuzzMutate/RandomIRBuilder.cpp | 16 ++++++- llvm/lib/IR/Verifier.cpp | 9 ++-- llvm/lib/Transforms/Utils/BasicBlockUtils.cpp | 6 ++- .../Transforms/LoopSimplifyCFG/mssa_term.ll | 46 +++++++++++++++++++ .../Analysis/BasicAliasAnalysisTest.cpp | 2 + .../unittests/Analysis/DomTreeUpdaterTest.cpp | 1 + llvm/unittests/Analysis/MemorySSATest.cpp | 33 +++++++++++++ .../Transforms/Utils/SSAUpdaterBulkTest.cpp | 2 + 9 files changed, 116 insertions(+), 15 deletions(-) create mode 100644 llvm/test/Transforms/LoopSimplifyCFG/mssa_term.ll 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; using const_succ_range = iterator_range; 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 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 getDominators(BasicBlock *BB) { std::vector 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 getDominators(BasicBlock *BB) { /// Return a vector of Blocks that is dominated by this block, excluding current /// block static std::vector getDominatees(BasicBlock *BB) { - DominatorTree DT(*BB->getParent()); + DominatorTree DT = getDomTree(*BB->getParent()); std::vector 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, 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(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, VerifierSupport { return false; } + // FIXME: It's really gross that we have to cast away constness here. + if (!F.empty()) + DT.recalculate(const_cast(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(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 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(B.CreateAdd(AddOp2, ConstantInt::get(I32Ty, 6))); auto *I3 = cast(B.CreateAdd(SubOp1, SubOp2)); auto *I4 = cast(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. From 6a826a476a798bb7a9c7f0101acba6c3a7d7c930 Mon Sep 17 00:00:00 2001 From: Alexis Engelke Date: Sun, 15 Mar 2026 08:16:29 +0000 Subject: [PATCH 2/4] clang-format --- llvm/include/llvm/IR/CFG.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/IR/CFG.h b/llvm/include/llvm/IR/CFG.h index e8158fdc243b1..c68c2f2e53d81 100644 --- a/llvm/include/llvm/IR/CFG.h +++ b/llvm/include/llvm/IR/CFG.h @@ -146,9 +146,7 @@ inline succ_iterator succ_begin(Instruction *I) { inline const_succ_iterator succ_begin(const Instruction *I) { return I->successors().begin(); } -inline succ_iterator succ_end(Instruction *I) { - return I->successors().end(); -} +inline succ_iterator succ_end(Instruction *I) { return I->successors().end(); } inline const_succ_iterator succ_end(const Instruction *I) { return I->successors().end(); } @@ -158,9 +156,7 @@ inline bool succ_empty(const Instruction *I) { inline unsigned succ_size(const Instruction *I) { return std::distance(succ_begin(I), succ_end(I)); } -inline succ_range successors(Instruction *I) { - return I->successors(); -} +inline succ_range successors(Instruction *I) { return I->successors(); } inline const_succ_range successors(const Instruction *I) { return I->successors(); } From c1bd502229b4bd73a615c507040b62a59447f4a8 Mon Sep 17 00:00:00 2001 From: Alexis Engelke Date: Sun, 15 Mar 2026 09:44:18 +0000 Subject: [PATCH 3/4] Fix OpenMP and Polly --- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 13 +++++++++++++ polly/lib/CodeGen/LoopGeneratorsGOMP.cpp | 3 +++ polly/lib/CodeGen/LoopGeneratorsKMP.cpp | 3 +++ 3 files changed, 19 insertions(+) diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index 4cbf0f9572be5..aa8a78f91eccc 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -5585,11 +5585,14 @@ OpenMPIRBuilder::applyStaticChunkedWorkshareLoop( Constant *One = ConstantInt::get(InternalIVTy, 1); Function *F = CLI->getFunction(); + // Blocks must have terminators. + auto *UI = new UnreachableInst(F->getContext(), CLI->getAfter()); FunctionAnalysisManager FAM; FAM.registerPass([]() { return DominatorTreeAnalysis(); }); FAM.registerPass([]() { return PassInstrumentationAnalysis(); }); LoopAnalysis LIA; LoopInfo &&LI = LIA.run(*F, FAM); + UI->eraseFromParent(); Loop *L = LI.getLoopFor(CLI->getHeader()); SmallVector LoopMDList; if (ChunkSize || DistScheduleChunkSize) @@ -6874,6 +6877,9 @@ void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop, Function *F = CanonicalLoop->getFunction(); + // Blocks must have terminators. + auto *UI = new UnreachableInst(Ctx, CanonicalLoop->getAfter()); + // TODO: We should not rely on pass manager. Currently we use pass manager // only for getting llvm::Loop which corresponds to given CanonicalLoopInfo // object. We should have a method which returns all blocks between @@ -6886,6 +6892,8 @@ void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop, LoopAnalysis LIA; LoopInfo &&LI = LIA.run(*F, FAM); + UI->eraseFromParent(); + Loop *L = LI.getLoopFor(CanonicalLoop->getHeader()); if (AlignedVars.size()) { InsertPointTy IP = Builder.saveIP(); @@ -7003,6 +7011,9 @@ static int32_t computeHeuristicUnrollFactor(CanonicalLoopInfo *CLI) { CodeGenOptLevel OptLevel = CodeGenOptLevel::Aggressive; std::unique_ptr TM = createTargetMachine(F, OptLevel); + // Blocks must have terminators. + auto *UI = new UnreachableInst(F->getContext(), CLI->getAfter()); + FunctionAnalysisManager FAM; FAM.registerPass([]() { return TargetLibraryAnalysis(); }); FAM.registerPass([]() { return AssumptionAnalysis(); }); @@ -7027,6 +7038,8 @@ static int32_t computeHeuristicUnrollFactor(CanonicalLoopInfo *CLI) { AssumptionCache &&AC = ACT.run(*F, FAM); OptimizationRemarkEmitter ORE{F}; + UI->eraseFromParent(); + Loop *L = LI.getLoopFor(CLI->getHeader()); assert(L && "Expecting CanonicalLoopInfo to be recognized as a loop"); diff --git a/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp b/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp index 82b255d2e43af..6c8b5d569665e 100644 --- a/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp +++ b/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp @@ -107,8 +107,11 @@ ParallelLoopGeneratorGOMP::createSubFn(Value *Stride, AllocaInst *StructData, // Create basic blocks. BasicBlock *HeaderBB = BasicBlock::Create(Context, "polly.par.setup", SubFn); + // Add terminator so that DT computation doesn't fail. + auto *UI = new UnreachableInst(Context, HeaderBB); SubFnDT = std::make_unique(*SubFn); SubFnLI = std::make_unique(*SubFnDT); + UI->eraseFromParent(); BasicBlock *ExitBB = BasicBlock::Create(Context, "polly.par.exit", SubFn); BasicBlock *CheckNextBB = diff --git a/polly/lib/CodeGen/LoopGeneratorsKMP.cpp b/polly/lib/CodeGen/LoopGeneratorsKMP.cpp index dfeea989f6c0b..2a8d5ea419cd5 100644 --- a/polly/lib/CodeGen/LoopGeneratorsKMP.cpp +++ b/polly/lib/CodeGen/LoopGeneratorsKMP.cpp @@ -131,8 +131,11 @@ ParallelLoopGeneratorKMP::createSubFn(Value *SequentialLoopStride, // Create basic blocks. BasicBlock *HeaderBB = BasicBlock::Create(Context, "polly.par.setup", SubFn); + // Add terminator so that DT computation doesn't fail. + auto *UI = new UnreachableInst(Context, HeaderBB); SubFnDT = std::make_unique(*SubFn); SubFnLI = std::make_unique(*SubFnDT); + UI->eraseFromParent(); BasicBlock *ExitBB = BasicBlock::Create(Context, "polly.par.exit", SubFn); BasicBlock *CheckNextBB = From 8cd562c59a74a6006bd0edd7d129fd5c49ca0222 Mon Sep 17 00:00:00 2001 From: Alexis Engelke Date: Sun, 15 Mar 2026 11:27:05 +0000 Subject: [PATCH 4/4] fix MLIR OpenMP --- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 24 +++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index aa8a78f91eccc..ca4e262144200 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -5586,13 +5586,17 @@ OpenMPIRBuilder::applyStaticChunkedWorkshareLoop( Function *F = CLI->getFunction(); // Blocks must have terminators. - auto *UI = new UnreachableInst(F->getContext(), CLI->getAfter()); + SmallVector UIs; + for (BasicBlock &BB : *F) + if (!BB.getTerminator()) + UIs.push_back(new UnreachableInst(F->getContext(), &BB)); FunctionAnalysisManager FAM; FAM.registerPass([]() { return DominatorTreeAnalysis(); }); FAM.registerPass([]() { return PassInstrumentationAnalysis(); }); LoopAnalysis LIA; LoopInfo &&LI = LIA.run(*F, FAM); - UI->eraseFromParent(); + for (Instruction *I : UIs) + I->eraseFromParent(); Loop *L = LI.getLoopFor(CLI->getHeader()); SmallVector LoopMDList; if (ChunkSize || DistScheduleChunkSize) @@ -6878,7 +6882,10 @@ void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop, Function *F = CanonicalLoop->getFunction(); // Blocks must have terminators. - auto *UI = new UnreachableInst(Ctx, CanonicalLoop->getAfter()); + SmallVector UIs; + for (BasicBlock &BB : *F) + if (!BB.getTerminator()) + UIs.push_back(new UnreachableInst(F->getContext(), &BB)); // TODO: We should not rely on pass manager. Currently we use pass manager // only for getting llvm::Loop which corresponds to given CanonicalLoopInfo @@ -6892,7 +6899,8 @@ void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop, LoopAnalysis LIA; LoopInfo &&LI = LIA.run(*F, FAM); - UI->eraseFromParent(); + for (Instruction *I : UIs) + I->eraseFromParent(); Loop *L = LI.getLoopFor(CanonicalLoop->getHeader()); if (AlignedVars.size()) { @@ -7012,7 +7020,10 @@ static int32_t computeHeuristicUnrollFactor(CanonicalLoopInfo *CLI) { std::unique_ptr TM = createTargetMachine(F, OptLevel); // Blocks must have terminators. - auto *UI = new UnreachableInst(F->getContext(), CLI->getAfter()); + SmallVector UIs; + for (BasicBlock &BB : *F) + if (!BB.getTerminator()) + UIs.push_back(new UnreachableInst(F->getContext(), &BB)); FunctionAnalysisManager FAM; FAM.registerPass([]() { return TargetLibraryAnalysis(); }); @@ -7038,7 +7049,8 @@ static int32_t computeHeuristicUnrollFactor(CanonicalLoopInfo *CLI) { AssumptionCache &&AC = ACT.run(*F, FAM); OptimizationRemarkEmitter ORE{F}; - UI->eraseFromParent(); + for (Instruction *I : UIs) + I->eraseFromParent(); Loop *L = LI.getLoopFor(CLI->getHeader()); assert(L && "Expecting CanonicalLoopInfo to be recognized as a loop");