Skip to content

Commit b4e4a17

Browse files
Pierre-vhbcahoon
authored andcommitted
[InferAddrSpaces] Correctly replace identical operands of insts (llvm#82610)
It's important for PHI nodes because if a PHI node has multiple edges coming from the same block, we can have the same incoming value multiple times in the list of incoming values. All of those need to be consistent (exact same Value*) otherwise verifier complains. Fixes SWDEV-445797 Change-Id: I8b462a2ddeb0b19e94ee866ff6582e3c4c9362b0
1 parent 9abb3a9 commit b4e4a17

File tree

2 files changed

+77
-5
lines changed

2 files changed

+77
-5
lines changed

llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,7 @@ bool InferAddressSpacesImpl::rewriteWithNewAddressSpaces(
12211221
Value::use_iterator I, E, Next;
12221222
for (I = V->use_begin(), E = V->use_end(); I != E;) {
12231223
Use &U = *I;
1224+
User *CurUser = U.getUser();
12241225

12251226
// Some users may see the same pointer operand in multiple operands. Skip
12261227
// to the next instruction.
@@ -1231,11 +1232,10 @@ bool InferAddressSpacesImpl::rewriteWithNewAddressSpaces(
12311232
// If V is used as the pointer operand of a compatible memory operation,
12321233
// sets the pointer operand to NewV. This replacement does not change
12331234
// the element type, so the resultant load/store is still valid.
1234-
U.set(NewV);
1235+
CurUser->replaceUsesOfWith(V, NewV);
12351236
continue;
12361237
}
12371238

1238-
User *CurUser = U.getUser();
12391239
// Skip if the current user is the new value itself.
12401240
if (CurUser == NewV)
12411241
continue;
@@ -1311,10 +1311,13 @@ bool InferAddressSpacesImpl::rewriteWithNewAddressSpaces(
13111311

13121312
while (isa<PHINode>(InsertPos))
13131313
++InsertPos;
1314-
U.set(new AddrSpaceCastInst(NewV, V->getType(), "", &*InsertPos));
1314+
// This instruction may contain multiple uses of V, update them all.
1315+
CurUser->replaceUsesOfWith(
1316+
V, new AddrSpaceCastInst(NewV, V->getType(), "", &*InsertPos));
13151317
} else {
1316-
U.set(ConstantExpr::getAddrSpaceCast(cast<Constant>(NewV),
1317-
V->getType()));
1318+
CurUser->replaceUsesOfWith(
1319+
V, ConstantExpr::getAddrSpaceCast(cast<Constant>(NewV),
1320+
V->getType()));
13181321
}
13191322
}
13201323
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a -S -passes=infer-address-spaces --verify-each %s | FileCheck %s
3+
4+
; Inst can use a value multiple time. When we're inserting an addrspacecast to flat,
5+
; it's important all the identical uses use an indentical replacement, especially
6+
; for PHIs.
7+
8+
define amdgpu_kernel void @test_phi() {
9+
; CHECK-LABEL: @test_phi(
10+
; CHECK-NEXT: entry:
11+
; CHECK-NEXT: [[LOADED_PTR:%.*]] = load ptr, ptr addrspace(4) null, align 8
12+
; CHECK-NEXT: [[TMP0:%.*]] = addrspacecast ptr [[LOADED_PTR]] to ptr addrspace(1)
13+
; CHECK-NEXT: br label [[BB0:%.*]]
14+
; CHECK: bb0:
15+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr i64, ptr addrspace(1) [[TMP0]], i64 3
16+
; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr addrspace(1) [[GEP]] to ptr
17+
; CHECK-NEXT: switch i32 0, label [[END:%.*]] [
18+
; CHECK-NEXT: i32 1, label [[END]]
19+
; CHECK-NEXT: i32 4, label [[END]]
20+
; CHECK-NEXT: i32 5, label [[BB1:%.*]]
21+
; CHECK-NEXT: ]
22+
; CHECK: bb1:
23+
; CHECK-NEXT: [[TMP2:%.*]] = load double, ptr addrspace(1) [[GEP]], align 16
24+
; CHECK-NEXT: br label [[END]]
25+
; CHECK: end:
26+
; CHECK-NEXT: [[RETVAL_SROA_0_0_I569_PH:%.*]] = phi ptr [ null, [[BB1]] ], [ [[TMP1]], [[BB0]] ], [ [[TMP1]], [[BB0]] ], [ [[TMP1]], [[BB0]] ]
27+
; CHECK-NEXT: ret void
28+
;
29+
entry:
30+
%loaded.ptr = load ptr, ptr addrspace(4) null, align 8
31+
br label %bb0
32+
33+
bb0:
34+
%gep = getelementptr i64, ptr %loaded.ptr, i64 3
35+
switch i32 0, label %end [
36+
i32 1, label %end
37+
i32 4, label %end
38+
i32 5, label %bb1
39+
]
40+
41+
bb1:
42+
%0 = load double, ptr %gep, align 16
43+
br label %end
44+
45+
end:
46+
%retval.sroa.0.0.i569.ph = phi ptr [ null, %bb1 ], [ %gep, %bb0 ], [ %gep, %bb0 ], [ %gep, %bb0 ]
47+
ret void
48+
}
49+
50+
declare void @uses_ptrs(ptr, ptr, ptr)
51+
52+
; We shouldn't treat PHIs differently, even other users should have the same treatment.
53+
; All occurences of %gep are replaced with an identical value.
54+
define amdgpu_kernel void @test_other() {
55+
; CHECK-LABEL: @test_other(
56+
; CHECK-NEXT: entry:
57+
; CHECK-NEXT: [[LOADED_PTR:%.*]] = load ptr, ptr addrspace(4) null, align 8
58+
; CHECK-NEXT: [[TMP0:%.*]] = addrspacecast ptr [[LOADED_PTR]] to ptr addrspace(1)
59+
; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr addrspace(1) [[TMP0]] to ptr
60+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr i64, ptr [[TMP1]], i64 3
61+
; CHECK-NEXT: call void @uses_ptrs(ptr [[GEP]], ptr [[GEP]], ptr [[GEP]])
62+
; CHECK-NEXT: ret void
63+
;
64+
entry:
65+
%loaded.ptr = load ptr, ptr addrspace(4) null, align 8
66+
%gep = getelementptr i64, ptr %loaded.ptr, i64 3
67+
call void @uses_ptrs(ptr %gep, ptr %gep, ptr %gep)
68+
ret void
69+
}

0 commit comments

Comments
 (0)