From: Alina Sbirlea Date: Wed, 9 Oct 2019 15:54:24 +0000 (+0000) Subject: [MemorySSA] Make the use of moveAllAfterMergeBlocks consistent. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7faa14a98bdc668822e99758f81a662f186ad35b;p=platform%2Fupstream%2Fllvm.git [MemorySSA] Make the use of moveAllAfterMergeBlocks consistent. Summary: The rule for the moveAllAfterMergeBlocks API si for all instructions from `From` to have been moved to `To`, while keeping the CFG edges (and block terminators) unchanged. Update all the callsites for moveAllAfterMergeBlocks to follow this. Pending follow-up: since the same behavior is needed everytime, merge all callsites into one. The common denominator may be the call to `MergeBlockIntoPredecessor`. Resolves PR43569. Reviewers: george.burgess.iv Subscribers: Prazek, sanjoy.google, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D68659 llvm-svn: 374177 --- diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp index d103c3a..78e197a 100644 --- a/llvm/lib/Analysis/MemorySSAUpdater.cpp +++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp @@ -1159,25 +1159,32 @@ void MemorySSAUpdater::moveAllAccesses(BasicBlock *From, BasicBlock *To, if (!Accs) return; + assert(Start->getParent() == To && "Incorrect Start instruction"); MemoryAccess *FirstInNew = nullptr; for (Instruction &I : make_range(Start->getIterator(), To->end())) if ((FirstInNew = MSSA->getMemoryAccess(&I))) break; - if (!FirstInNew) - return; + if (FirstInNew) { + auto *MUD = cast(FirstInNew); + do { + auto NextIt = ++MUD->getIterator(); + MemoryUseOrDef *NextMUD = (!Accs || NextIt == Accs->end()) + ? nullptr + : cast(&*NextIt); + MSSA->moveTo(MUD, To, MemorySSA::End); + // Moving MUD from Accs in the moveTo above, may delete Accs, so we need + // to retrieve it again. + Accs = MSSA->getWritableBlockAccesses(From); + MUD = NextMUD; + } while (MUD); + } - auto *MUD = cast(FirstInNew); - do { - auto NextIt = ++MUD->getIterator(); - MemoryUseOrDef *NextMUD = (!Accs || NextIt == Accs->end()) - ? nullptr - : cast(&*NextIt); - MSSA->moveTo(MUD, To, MemorySSA::End); - // Moving MUD from Accs in the moveTo above, may delete Accs, so we need to - // retrieve it again. - Accs = MSSA->getWritableBlockAccesses(From); - MUD = NextMUD; - } while (MUD); + // If all accesses were moved and only a trivial Phi remains, we try to remove + // that Phi. This is needed when From is going to be deleted. + auto *Defs = MSSA->getWritableBlockDefs(From); + if (Defs && !Defs->empty()) + if (auto *Phi = dyn_cast(&*Defs->begin())) + tryRemoveTrivialPhi(Phi); } void MemorySSAUpdater::moveAllAfterSpliceBlocks(BasicBlock *From, @@ -1193,7 +1200,7 @@ void MemorySSAUpdater::moveAllAfterSpliceBlocks(BasicBlock *From, void MemorySSAUpdater::moveAllAfterMergeBlocks(BasicBlock *From, BasicBlock *To, Instruction *Start) { - assert(From->getSinglePredecessor() == To && + assert(From->getUniquePredecessor() == To && "From block is expected to have a single predecessor (To)."); moveAllAccesses(From, To, Start); for (BasicBlock *Succ : successors(From)) diff --git a/llvm/lib/Transforms/Scalar/LoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/LoopUnswitch.cpp index 9938dd8..880f538 100644 --- a/llvm/lib/Transforms/Scalar/LoopUnswitch.cpp +++ b/llvm/lib/Transforms/Scalar/LoopUnswitch.cpp @@ -1629,15 +1629,27 @@ void LoopUnswitch::SimplifyCode(std::vector &Worklist, Loop *L) { ReplaceUsesOfWith(PN, PN->getIncomingValue(0), Worklist, L, LPM, MSSAU.get()); - // If Succ has any successors with PHI nodes, update them to have - // entries coming from Pred instead of Succ. - Succ->replaceAllUsesWith(Pred); + Instruction *STI = Succ->getTerminator(); + Instruction *Start = &*Succ->begin(); + // If there's nothing to move, mark the starting instruction as the last + // instruction in the block. + if (Start == STI) + Start = BI; // Move all of the successor contents from Succ to Pred. Pred->getInstList().splice(BI->getIterator(), Succ->getInstList(), - Succ->begin(), Succ->end()); + Succ->begin(), STI->getIterator()); if (MSSAU) - MSSAU->moveAllAfterMergeBlocks(Succ, Pred, BI); + MSSAU->moveAllAfterMergeBlocks(Succ, Pred, Start); + + // Move terminator instruction from Succ now, we're deleting BI below. + // FIXME: remove BI first might be more intuitive. + Pred->getInstList().splice(Pred->end(), Succ->getInstList()); + + // If Succ has any successors with PHI nodes, update them to have + // entries coming from Pred instead of Succ. + Succ->replaceAllUsesWith(Pred); + LPM->deleteSimpleAnalysisValue(BI, L); RemoveFromWorklist(BI, Worklist); BI->eraseFromParent(); diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp index 6e20ef2..b3dd3c9 100644 --- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp +++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp @@ -227,17 +227,29 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU, Updates.push_back({DominatorTree::Delete, PredBB, BB}); } - if (MSSAU) - MSSAU->moveAllAfterMergeBlocks(BB, PredBB, &*(BB->begin())); + Instruction *PTI = PredBB->getTerminator(); + Instruction *STI = BB->getTerminator(); + Instruction *Start = &*BB->begin(); + // If there's nothing to move, mark the starting instruction as the last + // instruction in the block. + if (Start == STI) + Start = PTI; - // Delete the unconditional branch from the predecessor... - PredBB->getInstList().pop_back(); + // Move all definitions in the successor to the predecessor... + PredBB->getInstList().splice(PTI->getIterator(), BB->getInstList(), + BB->begin(), STI->getIterator()); + + if (MSSAU) + MSSAU->moveAllAfterMergeBlocks(BB, PredBB, Start); // Make all PHI nodes that referred to BB now refer to Pred as their // source... BB->replaceAllUsesWith(PredBB); - // Move all definitions in the successor to the predecessor... + // Delete the unconditional branch from the predecessor... + PredBB->getInstList().pop_back(); + + // Move terminator instruction and add unreachable to now empty BB. PredBB->getInstList().splice(PredBB->end(), BB->getInstList()); new UnreachableInst(BB->getContext(), BB); @@ -274,11 +286,10 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU, "applying corresponding DTU updates."); DTU->applyUpdatesPermissive(Updates); DTU->deleteBB(BB); - } - - else { + } else { BB->eraseFromParent(); // Nuke BB if DTU is nullptr. } + return true; } diff --git a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp index 37389a6..4765aa11 100644 --- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp +++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp @@ -615,8 +615,13 @@ bool LoopRotate::simplifyLoopLatch(Loop *L) { LLVM_DEBUG(dbgs() << "Folding loop latch " << Latch->getName() << " into " << LastExit->getName() << "\n"); + Instruction *FirstLatchInst = &*Latch->begin(); + // If there's nothing to move, mark the starting instruction as the last + // instruction in the block. + if (FirstLatchInst == Jmp) + FirstLatchInst = BI; + // Hoist the instructions from Latch into LastExit. - Instruction *FirstLatchInst = &*(Latch->begin()); LastExit->getInstList().splice(BI->getIterator(), Latch->getInstList(), Latch->begin(), Jmp->getIterator()); diff --git a/llvm/test/Analysis/MemorySSA/pr43569.ll b/llvm/test/Analysis/MemorySSA/pr43569.ll new file mode 100644 index 0000000..c9c6845 --- /dev/null +++ b/llvm/test/Analysis/MemorySSA/pr43569.ll @@ -0,0 +1,49 @@ +; RUN: opt -pgo-kind=pgo-instr-gen-pipeline -aa-pipeline=default -passes="default" -enable-nontrivial-unswitch -S < %s | FileCheck %s +; REQUIRES: asserts + +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +@__profn_c = private constant [1 x i8] c"c" +@b = common dso_local global i32 0, align 4 +@a = common dso_local global i16 0, align 2 + +; CHECK-LABEL: @c() +; Function Attrs: nounwind uwtable +define dso_local void @c() #0 { +entry: + call void @llvm.instrprof.increment(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @__profn_c, i32 0, i32 0), i64 68269137, i32 3, i32 0) + br label %for.cond + +for.cond: ; preds = %for.end, %entry + call void @llvm.instrprof.increment(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @__profn_c, i32 0, i32 0), i64 68269137, i32 3, i32 1) + store i32 0, i32* @b, align 4 + br label %for.cond1 + +for.cond1: ; preds = %for.inc, %for.cond + %0 = load i32, i32* @b, align 4 + %1 = load i16, i16* @a, align 2 + %conv = sext i16 %1 to i32 + %cmp = icmp slt i32 %0, %conv + br i1 %cmp, label %for.body, label %for.end + +for.body: ; preds = %for.cond1 + call void @llvm.instrprof.increment(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @__profn_c, i32 0, i32 0), i64 68269137, i32 3, i32 2) + br label %for.inc + +for.inc: ; preds = %for.body + %2 = load i32, i32* @b, align 4 + %inc = add nsw i32 %2, 1 + store i32 %inc, i32* @b, align 4 + br label %for.cond1 + +for.end: ; preds = %for.cond1 + br label %for.cond +} + +; Function Attrs: nounwind +declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1 + +attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" } +attributes #1 = { nounwind } +