llvm.org GIT mirror llvm / a1d5c6d
[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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@355040 91177308-0d34-0410-b5e6-96231b3b80d8 Alina Sbirlea 5 months ago
5 changed file(s) with 177 addition(s) and 9 deletion(s). Raw diff Collapse all Expand all
268268 // Also make sure we skip ourselves to avoid self references.
269269 if (isa(U.getUser()) || U.getUser() == MD)
270270 continue;
271 // Defs are automatically unoptimized when the user is set to MD below,
272 // because the isOptimized() call will fail to find the same ID.
271273 U.set(MD);
272274 }
273275 }
274276
275277 // and that def is now our defining access.
276278 MD->setDefiningAccess(DefBefore);
279
280 // Remember the index where we may insert new phis below.
281 unsigned NewPhiIndex = InsertedPHIs.size();
277282
278283 SmallVector FixupList(InsertedPHIs.begin(), InsertedPHIs.end());
279284 if (!DefBeforeSameBlock) {
288293 // backwards to find the def. To make that work, we'd have to track whether
289294 // getDefRecursive only ever used the single predecessor case. These types
290295 // of paths also only exist in between CFG simplifications.
296
297 // If this is the first def in the block and this insert is in an arbitrary
298 // place, compute IDF and place phis.
299 auto Iter = MD->getDefsIterator();
300 ++Iter;
301 auto IterEnd = MSSA->getBlockDefs(MD->getBlock())->end();
302 if (Iter == IterEnd) {
303 ForwardIDFCalculator IDFs(*MSSA->DT);
304 SmallVector IDFBlocks;
305 SmallPtrSet DefiningBlocks;
306 DefiningBlocks.insert(MD->getBlock());
307 IDFs.setDefiningBlocks(DefiningBlocks);
308 IDFs.calculate(IDFBlocks);
309 SmallVector, 4> NewInsertedPHIs;
310 for (auto *BBIDF : IDFBlocks)
311 if (!MSSA->getMemoryAccess(BBIDF))
312 NewInsertedPHIs.push_back(MSSA->createMemoryPhi(BBIDF));
313
314 for (auto &MPhi : NewInsertedPHIs) {
315 auto *BBIDF = MPhi->getBlock();
316 for (auto *Pred : predecessors(BBIDF)) {
317 DenseMap> CachedPreviousDef;
318 MPhi->addIncoming(getPreviousDefFromEnd(Pred, CachedPreviousDef),
319 Pred);
320 }
321 }
322
323 // Re-take the index where we're adding the new phis, because the above
324 // call to getPreviousDefFromEnd, may have inserted into InsertedPHIs.
325 NewPhiIndex = InsertedPHIs.size();
326 for (auto &MPhi : NewInsertedPHIs) {
327 InsertedPHIs.push_back(&*MPhi);
328 FixupList.push_back(&*MPhi);
329 }
330 }
331
291332 FixupList.push_back(MD);
292333 }
334
335 // Remember the index where we stopped inserting new phis above, since the
336 // fixupDefs call in the loop below may insert more, that are already minimal.
337 unsigned NewPhiIndexEnd = InsertedPHIs.size();
293338
294339 while (!FixupList.empty()) {
295340 unsigned StartingPHISize = InsertedPHIs.size();
298343 // Put any new phis on the fixup list, and process them
299344 FixupList.append(InsertedPHIs.begin() + StartingPHISize, InsertedPHIs.end());
300345 }
346
347 // Optimize potentially non-minimal phis added in this method.
348 for (unsigned Idx = NewPhiIndex; Idx < NewPhiIndexEnd; ++Idx) {
349 if (auto *MPhi = cast_or_null(InsertedPHIs[Idx])) {
350 auto OperRange = MPhi->operands();
351 tryRemoveTrivialPhi(MPhi, OperRange);
352 }
353 }
354
301355 // Now that all fixups are done, rename all uses if we are asked.
302356 if (RenameUses) {
303357 SmallPtrSet Visited;
20672067 // stores in the loop.
20682068 Promoter.run(LoopUses);
20692069
2070 if (MSSAU && VerifyMemorySSA)
2071 MSSAU->getMemorySSA()->verifyMemorySSA();
20702072 // If the SSAUpdater didn't use the load in the preheader, just zap it now.
20712073 if (PreheaderLoad->use_empty())
20722074 eraseInstruction(*PreheaderLoad, *SafetyInfo, CurAST, MSSAU);
0 ; RUN: opt -licm -enable-mssa-loop-dependency -verify-memoryssa -S < %s | FileCheck %s
1 ; REQUIRES: asserts
2
3 target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
4 target triple = "systemz-unknown"
5
6 @g_3 = external dso_local local_unnamed_addr global i32, align 4
7 @g_57 = external dso_local local_unnamed_addr global i8, align 2
8 @g_82 = external dso_local global [8 x i16], align 2
9 @g_107 = external dso_local local_unnamed_addr global i32, align 4
10
11 define internal fastcc void @foo1() unnamed_addr{
12 ; CHECK-LABEL: @foo1()
13 entry:
14 %.pre.pre = load i32, i32* @g_3, align 4
15 br label %loop1
16
17 loop1:
18 %tmp0 = phi i32 [ undef, %entry ], [ %var18.lcssa, %loopexit ]
19 br label %preheader
20
21 preheader:
22 %indvars.iv = phi i64 [ 0, %loop1 ], [ %indvars.iv.next, %loop6 ]
23 %phi18 = phi i32 [ %tmp0, %loop1 ], [ 0, %loop6 ]
24 %phi87 = phi i32 [ 0, %loop1 ], [ %tmp7, %loop6 ]
25 %tmp1 = getelementptr inbounds [8 x i16], [8 x i16]* @g_82, i64 0, i64 %indvars.iv
26 %tmp2 = load i16, i16* %tmp1, align 2
27 %tmp3 = trunc i16 %tmp2 to i8
28 store i8 %tmp3, i8* @g_57, align 2
29 store i32 8, i32* @g_107, align 4
30 %tmp4 = icmp eq i32 %.pre.pre, 0
31 %spec.select = select i1 %tmp4, i32 %phi18, i32 14
32 %tmp5 = trunc i64 %indvars.iv to i32
33 switch i32 %spec.select, label %loopexit [
34 i32 0, label %loop6
35 i32 14, label %loop9
36 ]
37
38 loop6:
39 %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
40 %tmp7 = add nuw nsw i32 %phi87, 1
41 %tmp8 = icmp ult i64 %indvars.iv.next, 6
42 br i1 %tmp8, label %preheader, label %loop9
43
44 loop9:
45 %phi8.lcssa = phi i32 [ %tmp5, %preheader ], [ %tmp7, %loop6 ]
46 %tmp10 = trunc i32 %phi8.lcssa to i8
47 %tmp11 = tail call i16* @func_101(i16* getelementptr inbounds ([8 x i16], [8 x i16]* @g_82, i64 0, i64 6), i16* undef, i8 zeroext %tmp10)
48 unreachable
49
50 loopexit:
51 %var18.lcssa = phi i32 [ %phi18, %preheader ]
52 br label %loop1
53
54 }
55
56 declare dso_local i16* @func_101(i16*, i16*, i8) local_unnamed_addr
57
0 ; RUN: opt -licm -enable-mssa-loop-dependency -verify-memoryssa -S < %s | FileCheck %s
1 ; REQUIRES: asserts
2
3 target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
4 target triple = "systemz-unknown"
5
6 @g_120 = external dso_local local_unnamed_addr global [8 x [4 x [6 x i32]]], align 4
7 @g_185 = external dso_local local_unnamed_addr global i32, align 4
8 @g_329 = external dso_local local_unnamed_addr global i16, align 2
9
10 ; Function Attrs: norecurse noreturn nounwind
11 define dso_local void @func_65() local_unnamed_addr {
12 ; CHECK-LABEL: @func_65()
13 br label %1
14
15 ;
16 br label %2
17
18 ;
19 br label %3
20
21 ;
22 %storemerge = phi i32 [ 0, %2 ], [ %6, %5 ]
23 store i32 %storemerge, i32* @g_185, align 4
24 %4 = icmp ult i32 %storemerge, 2
25 br i1 %4, label %5, label %.thread.loopexit
26
27 ;
28 %6 = add i32 %storemerge, 1
29 %7 = zext i32 %6 to i64
30 %8 = getelementptr [8 x [4 x [6 x i32]]], [8 x [4 x [6 x i32]]]* @g_120, i64 0, i64 undef, i64 %7, i64 undef
31 %9 = load i32, i32* %8, align 4
32 %10 = icmp eq i32 %9, 0
33 br i1 %10, label %3, label %11
34
35 ;
36 %storemerge.lcssa4 = phi i32 [ %storemerge, %5 ]
37 %12 = icmp eq i32 %storemerge.lcssa4, 0
38 br i1 %12, label %.critedge, label %.thread.loopexit3
39
40 .critedge: ; preds = %11
41 store i16 0, i16* @g_329, align 2
42 br label %2
43
44 .thread.loopexit: ; preds = %3
45 br label %.thread
46
47 .thread.loopexit3: ; preds = %11
48 br label %.thread
49
50 .thread: ; preds = %.thread.loopexit3, %.thread.loopexit
51 br label %1
52 }
53
158158 MemoryAccess *LeftStoreAccess = Updater.createMemoryAccessInBB(
159159 LeftStore, nullptr, Left, MemorySSA::Beginning);
160160 Updater.insertDef(cast(LeftStoreAccess), false);
161 // We don't touch existing loads, so we need to create a new one to get a phi
161
162 // MemoryPHI should exist after adding LeftStore.
163 MP = MSSA.getMemoryAccess(Merge);
164 EXPECT_NE(MP, nullptr);
165
162166 // Add the second load
163167 B.SetInsertPoint(Merge, Merge->begin());
164168 LoadInst *SecondLoad = B.CreateLoad(B.getInt8Ty(), PointerArg);
165
166 // MemoryPHI should not already exist.
167 MP = MSSA.getMemoryAccess(Merge);
168 EXPECT_EQ(MP, nullptr);
169169
170170 // Create the load memory access
171171 MemoryUse *SecondLoadAccess = cast(Updater.createMemoryAccessInBB(
225225 Updater.createMemoryAccessInBB(SI, nullptr, Left, MemorySSA::Beginning);
226226 Updater.insertDef(cast(StoreAccess));
227227
228 // MemoryPHI should be created when inserting the def
229 MemoryPhi *MP = MSSA.getMemoryAccess(Merge);
230 EXPECT_NE(MP, nullptr);
231
228232 // Add the load
229233 B.SetInsertPoint(Merge, Merge->begin());
230234 LoadInst *LoadInst = B.CreateLoad(B.getInt8Ty(), PointerArg);
231
232 // MemoryPHI should not already exist.
233 MemoryPhi *MP = MSSA.getMemoryAccess(Merge);
234 EXPECT_EQ(MP, nullptr);
235235
236236 // Create the load memory acccess
237237 MemoryUse *LoadAccess = cast(Updater.createMemoryAccessInBB(