From 2c5e6646ef1300f6bff212d607e50057a637f271 Mon Sep 17 00:00:00 2001 From: Alina Sbirlea Date: Mon, 23 Sep 2019 23:50:16 +0000 Subject: [PATCH] [MemorySSA] Update Phi insertion. Summary: MemoryPhis may be needed following a Def insertion inthe IDF of all the new accesses added (phis + potentially a def). Ensure this also occurs when only the new MemoryPhis are the defining accesses. Note: The need for computing IDF here is because of new Phis added with edges incoming from unreachable code, Phis that had previously been simplified. The preferred solution is to not reintroduce such Phis. This patch is the needed fix while working on the preferred solution. Reviewers: george.burgess.iv Subscribers: Prazek, sanjoy.google, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D67927 llvm-svn: 372673 --- llvm/lib/Analysis/MemorySSAUpdater.cpp | 82 ++++++++++++++++----------------- llvm/test/Analysis/MemorySSA/pr43317.ll | 32 +++++++++++++ 2 files changed, 71 insertions(+), 43 deletions(-) create mode 100644 llvm/test/Analysis/MemorySSA/pr43317.ll diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp index 94417dc..bec811d 100644 --- a/llvm/lib/Analysis/MemorySSAUpdater.cpp +++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp @@ -313,10 +313,10 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) { // and that def is now our defining access. MD->setDefiningAccess(DefBefore); - // Remember the index where we may insert new phis below. - unsigned NewPhiIndex = InsertedPHIs.size(); - SmallVector FixupList(InsertedPHIs.begin(), InsertedPHIs.end()); + + SmallPtrSet DefiningBlocks; + if (!DefBeforeSameBlock) { // If there was a local def before us, we must have the same effect it // did. Because every may-def is the same, any phis/etc we would create, it @@ -335,53 +335,49 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) { auto Iter = MD->getDefsIterator(); ++Iter; auto IterEnd = MSSA->getBlockDefs(MD->getBlock())->end(); - if (Iter == IterEnd) { - ForwardIDFCalculator IDFs(*MSSA->DT); - SmallVector IDFBlocks; - SmallPtrSet DefiningBlocks; - for (const auto &VH : InsertedPHIs) - if (const auto *RealPHI = cast_or_null(VH)) - DefiningBlocks.insert(RealPHI->getBlock()); + if (Iter == IterEnd) DefiningBlocks.insert(MD->getBlock()); - IDFs.setDefiningBlocks(DefiningBlocks); - IDFs.calculate(IDFBlocks); - SmallVector, 4> NewInsertedPHIs; - for (auto *BBIDF : IDFBlocks) { - auto *MPhi = MSSA->getMemoryAccess(BBIDF); - if (!MPhi) { - MPhi = MSSA->createMemoryPhi(BBIDF); - NewInsertedPHIs.push_back(MPhi); - } - // Add the phis created into the IDF blocks to NonOptPhis, so they are - // not optimized out as trivial by the call to getPreviousDefFromEnd - // below. Once they are complete, all these Phis are added to the - // FixupList, and removed from NonOptPhis inside fixupDefs(). - // Existing Phis in IDF may need fixing as well, and potentially be - // trivial before this insertion, hence add all IDF Phis. See PR43044. - NonOptPhis.insert(MPhi); - } - for (auto &MPhi : NewInsertedPHIs) { - auto *BBIDF = MPhi->getBlock(); - for (auto *Pred : predecessors(BBIDF)) { - DenseMap> CachedPreviousDef; - MPhi->addIncoming(getPreviousDefFromEnd(Pred, CachedPreviousDef), - Pred); - } - } + FixupList.push_back(MD); + } - // Re-take the index where we're adding the new phis, because the above - // call to getPreviousDefFromEnd, may have inserted into InsertedPHIs. - NewPhiIndex = InsertedPHIs.size(); - for (auto &MPhi : NewInsertedPHIs) { - InsertedPHIs.push_back(&*MPhi); - FixupList.push_back(&*MPhi); - } + ForwardIDFCalculator IDFs(*MSSA->DT); + SmallVector IDFBlocks; + for (const auto &VH : InsertedPHIs) + if (const auto *RealPHI = cast_or_null(VH)) + DefiningBlocks.insert(RealPHI->getBlock()); + IDFs.setDefiningBlocks(DefiningBlocks); + IDFs.calculate(IDFBlocks); + SmallVector, 4> NewInsertedPHIs; + for (auto *BBIDF : IDFBlocks) { + auto *MPhi = MSSA->getMemoryAccess(BBIDF); + if (!MPhi) { + MPhi = MSSA->createMemoryPhi(BBIDF); + NewInsertedPHIs.push_back(MPhi); } + // Add the phis created into the IDF blocks to NonOptPhis, so they are not + // optimized out as trivial by the call to getPreviousDefFromEnd below. Once + // they are complete, all these Phis are added to the FixupList, and removed + // from NonOptPhis inside fixupDefs(). Existing Phis in IDF may need fixing + // as well, and potentially be trivial before this insertion, hence add all + // IDF Phis. See PR43044. + NonOptPhis.insert(MPhi); + } - FixupList.push_back(MD); + for (auto &MPhi : NewInsertedPHIs) { + auto *BBIDF = MPhi->getBlock(); + for (auto *Pred : predecessors(BBIDF)) { + DenseMap> CachedPreviousDef; + MPhi->addIncoming(getPreviousDefFromEnd(Pred, CachedPreviousDef), Pred); + } } + // Remember the index where we may insert new phis. + unsigned NewPhiIndex = InsertedPHIs.size(); + for (auto &MPhi : NewInsertedPHIs) { + InsertedPHIs.push_back(&*MPhi); + FixupList.push_back(&*MPhi); + } // Remember the index where we stopped inserting new phis above, since the // fixupDefs call in the loop below may insert more, that are already minimal. unsigned NewPhiIndexEnd = InsertedPHIs.size(); diff --git a/llvm/test/Analysis/MemorySSA/pr43317.ll b/llvm/test/Analysis/MemorySSA/pr43317.ll new file mode 100644 index 0000000..c7018b9 --- /dev/null +++ b/llvm/test/Analysis/MemorySSA/pr43317.ll @@ -0,0 +1,32 @@ +; RUN: opt -S -licm -enable-mssa-loop-dependency=true < %s | FileCheck %s +; REQUIRES: asserts +@v_274 = external dso_local global i64, align 1 +@v_295 = external dso_local global i16, align 1 +@v_335 = external dso_local global i32, align 1 + +; CHECK-LABEL: @main() +define dso_local void @main() { +entry: + store i32 undef, i32* @v_335, align 1 + br i1 undef, label %gate, label %exit + +nopredentry1: ; No predecessors! + br label %preinfiniteloop + +nopredentry2: ; No predecessors! + br label %gate + +gate: ; preds = %nopredentry2, %entry + br i1 undef, label %preinfiniteloop, label %exit + +preinfiniteloop: ; preds = %gate, %nopredentry1 + br label %infiniteloop + +infiniteloop: ; preds = %infiniteloop, %preinfiniteloop + store i16 undef, i16* @v_295, align 1 + br label %infiniteloop + +exit: ; preds = %gate, %entry + store i64 undef, i64* @v_274, align 1 + ret void +} -- 2.7.4