From fcfa7c5f92eb39486298e86b03e634e08a204c79 Mon Sep 17 00:00:00 2001 From: Alina Sbirlea Date: Wed, 27 Feb 2019 22:20:22 +0000 Subject: [MemorySSA] Make insertDef insert corresponding phi nodes. Summary: The original assumption for the insertDef method was that it would not materialize Defs out of no-where, hence it will not insert phis needed after inserting a Def. However, when cloning an instruction (use case used in LICM), we do materialize Defs "out of no-where". If the block receiving a Def has at least one other Def, then no processing is needed. If the block just received its first Def, we must check where Phi placement is needed. The only new usage of insertDef is in LICM, hence the trigger for the bug. But the original goal of the method also fails to apply for the move() method. If we move a Def from the entry point of a diamond to either the left or right blocks, then the merge block must add a phi. While this usecase does not currently occur, or may be viewed as an incorrect transformation, MSSA must behave corectly given the scenario. Resolves PR40749 and PR40754. Reviewers: george.burgess.iv Subscribers: sanjoy, jlebar, Prazek, jdoerfert, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D58652 llvm-svn: 355040 --- llvm/unittests/Analysis/MemorySSATest.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'llvm/unittests/Analysis/MemorySSATest.cpp') diff --git a/llvm/unittests/Analysis/MemorySSATest.cpp b/llvm/unittests/Analysis/MemorySSATest.cpp index c38124050f0..bfdf37ee190 100644 --- a/llvm/unittests/Analysis/MemorySSATest.cpp +++ b/llvm/unittests/Analysis/MemorySSATest.cpp @@ -159,15 +159,15 @@ TEST_F(MemorySSATest, CreateLoadsAndStoreUpdater) { MemoryAccess *LeftStoreAccess = Updater.createMemoryAccessInBB( LeftStore, nullptr, Left, MemorySSA::Beginning); Updater.insertDef(cast(LeftStoreAccess), false); - // We don't touch existing loads, so we need to create a new one to get a phi + + // MemoryPHI should exist after adding LeftStore. + MP = MSSA.getMemoryAccess(Merge); + EXPECT_NE(MP, nullptr); + // Add the second load B.SetInsertPoint(Merge, Merge->begin()); LoadInst *SecondLoad = B.CreateLoad(B.getInt8Ty(), PointerArg); - // MemoryPHI should not already exist. - MP = MSSA.getMemoryAccess(Merge); - EXPECT_EQ(MP, nullptr); - // Create the load memory access MemoryUse *SecondLoadAccess = cast(Updater.createMemoryAccessInBB( SecondLoad, nullptr, Merge, MemorySSA::Beginning)); @@ -226,14 +226,14 @@ TEST_F(MemorySSATest, CreateALoadUpdater) { Updater.createMemoryAccessInBB(SI, nullptr, Left, MemorySSA::Beginning); Updater.insertDef(cast(StoreAccess)); + // MemoryPHI should be created when inserting the def + MemoryPhi *MP = MSSA.getMemoryAccess(Merge); + EXPECT_NE(MP, nullptr); + // Add the load B.SetInsertPoint(Merge, Merge->begin()); LoadInst *LoadInst = B.CreateLoad(B.getInt8Ty(), PointerArg); - // MemoryPHI should not already exist. - MemoryPhi *MP = MSSA.getMemoryAccess(Merge); - EXPECT_EQ(MP, nullptr); - // Create the load memory acccess MemoryUse *LoadAccess = cast(Updater.createMemoryAccessInBB( LoadInst, nullptr, Merge, MemorySSA::Beginning)); -- cgit v1.2.3