Skip to content

Commit 0363c3b

Browse files
alinasalinas
committed
[MemorySSA] Check that block is reachable when adding phis.
Summary: Originally the insertDef method was only used when building MemorySSA, and was limiting the number of Phi nodes that it created. Now it's used for updates as well, and it can create additional Phis needed for correctness. Make sure no Phis are created in unreachable blocks (condition met during MSSA build), otherwise the renamePass will find a null DTNode. Resolves PR41640. Reviewers: george.burgess.iv Subscribers: jlebar, Prazek, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D61410 llvm-svn: 359845
1 parent f7d3048 commit 0363c3b

File tree

3 files changed

+109
-1
lines changed

3 files changed

+109
-1
lines changed

‎llvm/lib/Analysis/MemorySSA.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -1125,6 +1125,8 @@ MemoryAccess *MemorySSA::renameBlock(BasicBlock *BB, MemoryAccess *IncomingVal,
11251125
void MemorySSA::renamePass(DomTreeNode *Root, MemoryAccess *IncomingVal,
11261126
SmallPtrSetImpl<BasicBlock *> &Visited,
11271127
bool SkipVisited, bool RenameAllUses) {
1128+
assert(Root && "Trying to rename accesses in an unreachable block");
1129+
11281130
SmallVector<RenamePassData, 32> WorkStack;
11291131
// Skip everything if we already renamed this block and we are skipping.
11301132
// Note: You can't sink this into the if, because we need it to occur

‎llvm/lib/Analysis/MemorySSAUpdater.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ MemoryAccess *MemorySSAUpdater::getPreviousDefRecursive(
7272
// potential phi node. This will insert phi nodes if we cycle in order to
7373
// break the cycle and have an operand.
7474
for (auto *Pred : predecessors(BB))
75-
PhiOps.push_back(getPreviousDefFromEnd(Pred, CachedPreviousDef));
75+
if (MSSA->DT->isReachableFromEntry(Pred))
76+
PhiOps.push_back(getPreviousDefFromEnd(Pred, CachedPreviousDef));
77+
else
78+
PhiOps.push_back(MSSA->getLiveOnEntryDef());
7679

7780
// Now try to simplify the ops to avoid placing a phi.
7881
// This may return null if we never created a phi yet, that's okay

‎llvm/test/Analysis/MemorySSA/pr41640.ll

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
; RUN: opt -S -licm -enable-mssa-loop-dependency %s | FileCheck %s
2+
; REQUIRES: asserts
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 = "s390x-ibm-linux"
5+
6+
@g_248 = external dso_local local_unnamed_addr global i32, align 4
7+
@g_976 = external dso_local global i64, align 8
8+
@g_1087 = external dso_local global i32**, align 8
9+
10+
; CHECK-LABEL: @f1()
11+
define dso_local fastcc void @f1() unnamed_addr #0 {
12+
label0:
13+
br i1 undef, label %thread-pre-split.i.preheader, label %label5
14+
15+
thread-pre-split.i.preheader: ; preds = %label0
16+
br label %thread-pre-split.i
17+
18+
thread-pre-split.i.us: ; preds = %.critedge1.i.us
19+
br i1 undef, label %.preheader.i.us.preheader, label %label2
20+
21+
.preheader.i.us.preheader: ; preds = %thread-pre-split.i.us
22+
br label %.preheader.i.us
23+
24+
.preheader.i.us: ; preds = %._crit_edge.i.us, %.preheader.i.us.preheader
25+
br i1 undef, label %.lr.ph.i.us, label %._crit_edge.i.us
26+
27+
.lr.ph.i.us: ; preds = %.preheader.i.us
28+
br label %label1
29+
30+
label1: ; preds = %label1, %.lr.ph.i.us
31+
br i1 undef, label %label1, label %._crit_edge.i.us
32+
33+
._crit_edge.i.us: ; preds = %label1, %.preheader.i.us
34+
br i1 undef, label %.preheader.i.us, label %._crit_edge5.i.us
35+
36+
._crit_edge5.i.us: ; preds = %._crit_edge.i.us
37+
br label %label2
38+
39+
label2: ; preds = %._crit_edge5.i.us, %thread-pre-split.i.us
40+
tail call void @foo16()
41+
br i1 undef, label %.lr.ph8.i.us.preheader, label %label4
42+
43+
.lr.ph8.i.us.preheader: ; preds = %label2
44+
br label %.lr.ph8.i.us
45+
46+
.lr.ph8.i.us: ; preds = %.lr.ph8.i.us, %.lr.ph8.i.us.preheader
47+
%tmp3 = load volatile i64, i64* @g_976, align 8
48+
br i1 undef, label %.lr.ph8.i.us, label %._crit_edge9.i.us
49+
50+
._crit_edge9.i.us: ; preds = %.lr.ph8.i.us
51+
br label %label4
52+
53+
label4: ; preds = %._crit_edge9.i.us, %label2
54+
br i1 true, label %f9.exit, label %.critedge1.i.us
55+
56+
.critedge1.i.us: ; preds = %label4
57+
br i1 undef, label %thread-pre-split.i.us, label %f9.exit
58+
59+
label5: ; preds = %label0
60+
unreachable
61+
62+
thread-pre-split.i: ; preds = %.critedge1.i, %thread-pre-split.i.preheader
63+
br i1 undef, label %.preheader.i.preheader, label %.critedge1.i
64+
65+
.preheader.i.preheader: ; preds = %thread-pre-split.i
66+
br label %.preheader.i
67+
68+
.preheader.i: ; preds = %._crit_edge.i, %.preheader.i.preheader
69+
br i1 undef, label %.lr.ph.i, label %._crit_edge.i
70+
71+
.lr.ph.i: ; preds = %.preheader.i
72+
br label %label6
73+
74+
label6: ; preds = %label6, %.lr.ph.i
75+
br i1 undef, label %label6, label %._crit_edge.i.loopexit
76+
77+
._crit_edge.i.loopexit: ; preds = %label6
78+
br label %._crit_edge.i
79+
80+
._crit_edge.i: ; preds = %._crit_edge.i.loopexit, %.preheader.i
81+
br i1 undef, label %.preheader.i, label %._crit_edge5.i
82+
83+
._crit_edge5.i: ; preds = %._crit_edge.i
84+
br label %.critedge1.i
85+
86+
.critedge1.i: ; preds = %._crit_edge5.i, %thread-pre-split.i
87+
%tmp7 = load i32, i32* @g_248, align 4
88+
%tmp8 = xor i32 %tmp7, 55987
89+
store i32 %tmp8, i32* @g_248, align 4
90+
br i1 undef, label %thread-pre-split.i, label %f9.exit.loopexit
91+
92+
f9.exit.loopexit: ; preds = %.critedge1.i
93+
br label %f9.exit
94+
95+
f9.exit: ; preds = %f9.exit.loopexit, %.critedge1.i.us, %label4
96+
%tmp9 = load volatile i32**, i32*** @g_1087, align 8
97+
ret void
98+
}
99+
100+
declare dso_local void @foo16() local_unnamed_addr #1
101+
102+
attributes #0 = { "target-features"="+transactional-execution,+vector" }
103+
attributes #1 = { "use-soft-float"="false" }

0 commit comments

Comments
 (0)