Skip to content

Commit f73ba0f

Browse files
rotaterighttstellar
rotateright
authored and
tstellar
committed
[SimplifyCFG] avoid illegal phi with both poison and undef
In the example based on: https://llvm.org/PR49218 ...we are crashing because poison is a subclass of undef, so we merge blocks and create: PHI node has multiple entries for the same basic block with different incoming values! %k3 = phi i64 [ poison, %entry ], [ %k3, %g ], [ undef, %entry ] If both poison and undef values are incoming, we soften the poison values to undef. Differential Revision: https://reviews.llvm.org/D97495 (cherry picked from commit 356cdab)
1 parent 692808e commit f73ba0f

File tree

2 files changed

+223
-1
lines changed

2 files changed

+223
-1
lines changed

‎llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,17 +918,39 @@ static void gatherIncomingValuesToPhi(PHINode *PN,
918918
/// \param IncomingValues A map from block to value.
919919
static void replaceUndefValuesInPhi(PHINode *PN,
920920
const IncomingValueMap &IncomingValues) {
921+
SmallVector<unsigned> TrueUndefOps;
921922
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
922923
Value *V = PN->getIncomingValue(i);
923924

924925
if (!isa<UndefValue>(V)) continue;
925926

926927
BasicBlock *BB = PN->getIncomingBlock(i);
927928
IncomingValueMap::const_iterator It = IncomingValues.find(BB);
928-
if (It == IncomingValues.end()) continue;
929929

930+
// Keep track of undef/poison incoming values. Those must match, so we fix
931+
// them up below if needed.
932+
// Note: this is conservatively correct, but we could try harder and group
933+
// the undef values per incoming basic block.
934+
if (It == IncomingValues.end()) {
935+
TrueUndefOps.push_back(i);
936+
continue;
937+
}
938+
939+
// There is a defined value for this incoming block, so map this undef
940+
// incoming value to the defined value.
930941
PN->setIncomingValue(i, It->second);
931942
}
943+
944+
// If there are both undef and poison values incoming, then convert those
945+
// values to undef. It is invalid to have different values for the same
946+
// incoming block.
947+
unsigned PoisonCount = count_if(TrueUndefOps, [&](unsigned i) {
948+
return isa<PoisonValue>(PN->getIncomingValue(i));
949+
});
950+
if (PoisonCount != 0 && PoisonCount != TrueUndefOps.size()) {
951+
for (unsigned i : TrueUndefOps)
952+
PN->setIncomingValue(i, UndefValue::get(PN->getType()));
953+
}
932954
}
933955

934956
/// Replace a value flowing from a block to a phi with

‎llvm/test/Transforms/SimplifyCFG/poison-merge.ll

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -S -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -keep-loops=0 < %s | FileCheck %s
3+
4+
; Merge 2 undefined incoming values.
5+
6+
define i32 @undef_merge(i32 %x) {
7+
; CHECK-LABEL: @undef_merge(
8+
; CHECK-NEXT: entry:
9+
; CHECK-NEXT: switch i32 [[X:%.*]], label [[EXIT:%.*]] [
10+
; CHECK-NEXT: i32 4, label [[G:%.*]]
11+
; CHECK-NEXT: i32 12, label [[G]]
12+
; CHECK-NEXT: ]
13+
; CHECK: g:
14+
; CHECK-NEXT: [[K3:%.*]] = phi i64 [ undef, [[ENTRY:%.*]] ], [ [[K3]], [[G]] ], [ undef, [[ENTRY]] ]
15+
; CHECK-NEXT: br label [[G]]
16+
; CHECK: exit:
17+
; CHECK-NEXT: ret i32 undef
18+
;
19+
entry:
20+
switch i32 %x, label %exit [
21+
i32 4, label %loop
22+
i32 12, label %g
23+
]
24+
25+
loop:
26+
%k2 = phi i64 [ %k3, %g ], [ undef, %entry ]
27+
br label %g
28+
29+
g:
30+
%k3 = phi i64 [ %k2, %loop ], [ undef, %entry ]
31+
br label %loop
32+
33+
exit:
34+
ret i32 undef
35+
}
36+
37+
; Merge 2 poison incoming values.
38+
39+
define i32 @poison_merge(i32 %x) {
40+
; CHECK-LABEL: @poison_merge(
41+
; CHECK-NEXT: entry:
42+
; CHECK-NEXT: switch i32 [[X:%.*]], label [[EXIT:%.*]] [
43+
; CHECK-NEXT: i32 4, label [[G:%.*]]
44+
; CHECK-NEXT: i32 12, label [[G]]
45+
; CHECK-NEXT: ]
46+
; CHECK: g:
47+
; CHECK-NEXT: [[K3:%.*]] = phi i64 [ poison, [[ENTRY:%.*]] ], [ [[K3]], [[G]] ], [ poison, [[ENTRY]] ]
48+
; CHECK-NEXT: br label [[G]]
49+
; CHECK: exit:
50+
; CHECK-NEXT: ret i32 undef
51+
;
52+
entry:
53+
switch i32 %x, label %exit [
54+
i32 4, label %loop
55+
i32 12, label %g
56+
]
57+
58+
loop:
59+
%k2 = phi i64 [ %k3, %g ], [ poison, %entry ]
60+
br label %g
61+
62+
g:
63+
%k3 = phi i64 [ %k2, %loop ], [ poison, %entry ]
64+
br label %loop
65+
66+
exit:
67+
ret i32 undef
68+
}
69+
70+
; Merge equal defined incoming values.
71+
72+
define i32 @defined_merge(i32 %x) {
73+
; CHECK-LABEL: @defined_merge(
74+
; CHECK-NEXT: entry:
75+
; CHECK-NEXT: switch i32 [[X:%.*]], label [[EXIT:%.*]] [
76+
; CHECK-NEXT: i32 4, label [[G:%.*]]
77+
; CHECK-NEXT: i32 12, label [[G]]
78+
; CHECK-NEXT: ]
79+
; CHECK: g:
80+
; CHECK-NEXT: [[K3:%.*]] = phi i64 [ 42, [[ENTRY:%.*]] ], [ [[K3]], [[G]] ], [ 42, [[ENTRY]] ]
81+
; CHECK-NEXT: br label [[G]]
82+
; CHECK: exit:
83+
; CHECK-NEXT: ret i32 undef
84+
;
85+
entry:
86+
switch i32 %x, label %exit [
87+
i32 4, label %loop
88+
i32 12, label %g
89+
]
90+
91+
loop:
92+
%k2 = phi i64 [ %k3, %g ], [ 42, %entry ]
93+
br label %g
94+
95+
g:
96+
%k3 = phi i64 [ %k2, %loop ], [ 42, %entry ]
97+
br label %loop
98+
99+
exit:
100+
ret i32 undef
101+
}
102+
103+
; Merge defined and undef incoming values.
104+
105+
define i32 @defined_and_undef_merge(i32 %x) {
106+
; CHECK-LABEL: @defined_and_undef_merge(
107+
; CHECK-NEXT: entry:
108+
; CHECK-NEXT: switch i32 [[X:%.*]], label [[EXIT:%.*]] [
109+
; CHECK-NEXT: i32 4, label [[G:%.*]]
110+
; CHECK-NEXT: i32 12, label [[G]]
111+
; CHECK-NEXT: ]
112+
; CHECK: g:
113+
; CHECK-NEXT: [[K3:%.*]] = phi i64 [ 42, [[ENTRY:%.*]] ], [ [[K3]], [[G]] ], [ 42, [[ENTRY]] ]
114+
; CHECK-NEXT: br label [[G]]
115+
; CHECK: exit:
116+
; CHECK-NEXT: ret i32 undef
117+
;
118+
entry:
119+
switch i32 %x, label %exit [
120+
i32 4, label %loop
121+
i32 12, label %g
122+
]
123+
124+
loop:
125+
%k2 = phi i64 [ %k3, %g ], [ undef, %entry ]
126+
br label %g
127+
128+
g:
129+
%k3 = phi i64 [ %k2, %loop ], [ 42, %entry ]
130+
br label %loop
131+
132+
exit:
133+
ret i32 undef
134+
}
135+
136+
; Merge defined and poison incoming values.
137+
138+
define i32 @defined_and_poison_merge(i32 %x) {
139+
; CHECK-LABEL: @defined_and_poison_merge(
140+
; CHECK-NEXT: entry:
141+
; CHECK-NEXT: switch i32 [[X:%.*]], label [[EXIT:%.*]] [
142+
; CHECK-NEXT: i32 4, label [[G:%.*]]
143+
; CHECK-NEXT: i32 12, label [[G]]
144+
; CHECK-NEXT: ]
145+
; CHECK: g:
146+
; CHECK-NEXT: [[K3:%.*]] = phi i64 [ 42, [[ENTRY:%.*]] ], [ [[K3]], [[G]] ], [ 42, [[ENTRY]] ]
147+
; CHECK-NEXT: br label [[G]]
148+
; CHECK: exit:
149+
; CHECK-NEXT: ret i32 undef
150+
;
151+
entry:
152+
switch i32 %x, label %exit [
153+
i32 4, label %loop
154+
i32 12, label %g
155+
]
156+
157+
loop:
158+
%k2 = phi i64 [ %k3, %g ], [ poison, %entry ]
159+
br label %g
160+
161+
g:
162+
%k3 = phi i64 [ %k2, %loop ], [ 42, %entry ]
163+
br label %loop
164+
165+
exit:
166+
ret i32 undef
167+
}
168+
169+
; Do not crash trying to merge poison and undef into a single phi.
170+
171+
define i32 @PR49218(i32 %x) {
172+
; CHECK-LABEL: @PR49218(
173+
; CHECK-NEXT: entry:
174+
; CHECK-NEXT: switch i32 [[X:%.*]], label [[EXIT:%.*]] [
175+
; CHECK-NEXT: i32 4, label [[G:%.*]]
176+
; CHECK-NEXT: i32 12, label [[G]]
177+
; CHECK-NEXT: ]
178+
; CHECK: g:
179+
; CHECK-NEXT: [[K3:%.*]] = phi i64 [ undef, [[ENTRY:%.*]] ], [ [[K3]], [[G]] ], [ undef, [[ENTRY]] ]
180+
; CHECK-NEXT: br label [[G]]
181+
; CHECK: exit:
182+
; CHECK-NEXT: ret i32 undef
183+
;
184+
entry:
185+
switch i32 %x, label %exit [
186+
i32 4, label %loop
187+
i32 12, label %g
188+
]
189+
190+
loop:
191+
%k2 = phi i64 [ %k3, %g ], [ undef, %entry ]
192+
br label %g
193+
194+
g:
195+
%k3 = phi i64 [ %k2, %loop ], [ poison, %entry ]
196+
br label %loop
197+
198+
exit:
199+
ret i32 undef
200+
}

0 commit comments

Comments
 (0)