Skip to content

Commit db4cbe5

Browse files
VyacheslavLevytskyyVyacheslavLevytskyy
authored
[SPIR-V] Fix generation of invalid SPIR-V in cases of of bitcasts between pointers and multiple null pointers used in the input LLVM IR (#118298)
This PR resolved the following issues: (1) There are rare but possible cases when there are bitcasts between pointers intertwined in a sophisticated way with loads, stores, function calls and other instructions that are part of type deduction. In this case we must account for inserted bitcasts between pointers rather than just ignore them. (2) Null pointers have the same constant representation but different types. Type info from Intrinsic::spv_track_constant() refers to the opaque (untyped) pointer, so that each MF/v-reg pair would fall into the same Const record in Duplicate Tracker and would be represented by a single OpConstantNull instruction, unless we use precise pointee type info. We must be able to distinguish one constant (null) pointer from another to avoid generating invalid code with inconsistent types of operands.
1 parent 10223c7 commit db4cbe5

File tree

4 files changed

+4959
-23
lines changed

4 files changed

+4959
-23
lines changed

‎llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ void SPIRVEmitIntrinsics::propagateElemType(
475475
DenseMap<Function *, CallInst *> Ptrcasts;
476476
SmallVector<User *> Users(Op->users());
477477
for (auto *U : Users) {
478-
if (!isa<Instruction>(U) || isa<BitCastInst>(U) || isSpvIntrinsic(U))
478+
if (!isa<Instruction>(U) || isSpvIntrinsic(U))
479479
continue;
480480
if (!VisitedSubst.insert(std::make_pair(U, Op)).second)
481481
continue;
@@ -506,7 +506,7 @@ void SPIRVEmitIntrinsics::propagateElemTypeRec(
506506
return;
507507
SmallVector<User *> Users(Op->users());
508508
for (auto *U : Users) {
509-
if (!isa<Instruction>(U) || isa<BitCastInst>(U) || isSpvIntrinsic(U))
509+
if (!isa<Instruction>(U) || isSpvIntrinsic(U))
510510
continue;
511511
if (!VisitedSubst.insert(std::make_pair(U, Op)).second)
512512
continue;
@@ -958,6 +958,14 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
958958
return;
959959
Uncomplete = isTodoType(I);
960960
Ops.push_back(std::make_pair(Ref->getPointerOperand(), 0));
961+
} else if (auto *Ref = dyn_cast<BitCastInst>(I)) {
962+
if (!isPointerTy(I->getType()))
963+
return;
964+
KnownElemTy = GR->findDeducedElementType(I);
965+
if (!KnownElemTy)
966+
return;
967+
Uncomplete = isTodoType(I);
968+
Ops.push_back(std::make_pair(Ref->getOperand(0), 0));
961969
} else if (auto *Ref = dyn_cast<GetElementPtrInst>(I)) {
962970
if (GR->findDeducedElementType(Ref->getPointerOperand()))
963971
return;
@@ -1030,7 +1038,6 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
10301038
}
10311039
}
10321040
}
1033-
TypeValidated.insert(I);
10341041
// Non-recursive update of types in the function uncomplete returns.
10351042
// This may happen just once per a function, the latch is a pair of
10361043
// findDeducedElementType(F) / addDeducedElementType(F, ...).
@@ -1043,6 +1050,7 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(
10431050
} else if (UncompleteRets) {
10441051
UncompleteRets->insert(I);
10451052
}
1053+
TypeValidated.insert(I);
10461054
return;
10471055
}
10481056
Uncomplete = isTodoType(CurrF);
@@ -1369,10 +1377,6 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
13691377
Instruction *I, Value *Pointer, Type *ExpectedElementType,
13701378
unsigned OperandToReplace, IRBuilder<> &B) {
13711379
TypeValidated.insert(I);
1372-
// If Pointer is the result of nop BitCastInst (ptr -> ptr), use the source
1373-
// pointer instead. The BitCastInst should be later removed when visited.
1374-
while (BitCastInst *BC = dyn_cast<BitCastInst>(Pointer))
1375-
Pointer = BC->getOperand(0);
13761380

13771381
// Do not emit spv_ptrcast if Pointer's element type is ExpectedElementType
13781382
Type *PointerElemTy = deduceElementTypeHelper(Pointer, false);
@@ -1759,8 +1763,7 @@ bool SPIRVEmitIntrinsics::insertAssignPtrTypeIntrs(Instruction *I,
17591763
IRBuilder<> &B,
17601764
bool UnknownElemTypeI8) {
17611765
reportFatalOnTokenType(I);
1762-
if (!isPointerTy(I->getType()) || !requireAssignType(I) ||
1763-
isa<BitCastInst>(I))
1766+
if (!isPointerTy(I->getType()) || !requireAssignType(I))
17641767
return false;
17651768

17661769
setInsertPointAfterDef(B, I);
@@ -1861,8 +1864,9 @@ void SPIRVEmitIntrinsics::insertSpirvDecorations(Instruction *I,
18611864
void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
18621865
IRBuilder<> &B) {
18631866
auto *II = dyn_cast<IntrinsicInst>(I);
1864-
if (II && II->getIntrinsicID() == Intrinsic::spv_const_composite &&
1865-
TrackConstants) {
1867+
bool IsConstComposite =
1868+
II && II->getIntrinsicID() == Intrinsic::spv_const_composite;
1869+
if (IsConstComposite && TrackConstants) {
18661870
setInsertPointAfterDef(B, I);
18671871
auto t = AggrConsts.find(I);
18681872
assert(t != AggrConsts.end());
@@ -1886,12 +1890,27 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
18861890
: B.SetInsertPoint(I);
18871891
BPrepared = true;
18881892
}
1893+
Type *OpTy = Op->getType();
18891894
Value *OpTyVal = Op;
1890-
if (Op->getType()->isTargetExtTy())
1891-
OpTyVal = PoisonValue::get(Op->getType());
1892-
auto *NewOp = buildIntrWithMD(Intrinsic::spv_track_constant,
1893-
{Op->getType(), OpTyVal->getType()}, Op,
1894-
OpTyVal, {}, B);
1895+
if (OpTy->isTargetExtTy())
1896+
OpTyVal = PoisonValue::get(OpTy);
1897+
CallInst *NewOp =
1898+
buildIntrWithMD(Intrinsic::spv_track_constant,
1899+
{OpTy, OpTyVal->getType()}, Op, OpTyVal, {}, B);
1900+
Type *OpElemTy = nullptr;
1901+
if (!IsConstComposite && isPointerTy(OpTy) &&
1902+
(OpElemTy = GR->findDeducedElementType(Op)) != nullptr &&
1903+
OpElemTy != IntegerType::getInt8Ty(I->getContext())) {
1904+
buildAssignPtr(B, IntegerType::getInt8Ty(I->getContext()), NewOp);
1905+
SmallVector<Type *, 2> Types = {OpTy, OpTy};
1906+
SmallVector<Value *, 2> Args = {
1907+
NewOp, buildMD(PoisonValue::get(OpElemTy)),
1908+
B.getInt32(getPointerAddressSpace(OpTy))};
1909+
CallInst *PtrCasted =
1910+
B.CreateIntrinsic(Intrinsic::spv_ptrcast, {Types}, Args);
1911+
buildAssignPtr(B, OpElemTy, PtrCasted);
1912+
NewOp = PtrCasted;
1913+
}
18951914
I->setOperand(OpNo, NewOp);
18961915
}
18971916
}
@@ -2022,8 +2041,16 @@ void SPIRVEmitIntrinsics::processParamTypes(Function *F, IRBuilder<> &B) {
20222041
if (!isUntypedPointerTy(Arg->getType()))
20232042
continue;
20242043
Type *ElemTy = GR->findDeducedElementType(Arg);
2025-
if (!ElemTy && (ElemTy = deduceFunParamElementType(F, OpIdx)) != nullptr)
2026-
buildAssignPtr(B, ElemTy, Arg);
2044+
if (!ElemTy && (ElemTy = deduceFunParamElementType(F, OpIdx)) != nullptr) {
2045+
if (CallInst *AssignCI = GR->findAssignPtrTypeInstr(Arg)) {
2046+
DenseSet<std::pair<Value *, Value *>> VisitedSubst;
2047+
updateAssignType(AssignCI, Arg, PoisonValue::get(ElemTy));
2048+
propagateElemType(Arg, IntegerType::getInt8Ty(F->getContext()),
2049+
VisitedSubst);
2050+
} else {
2051+
buildAssignPtr(B, ElemTy, Arg);
2052+
}
2053+
}
20272054
}
20282055
}
20292056

‎llvm/test/CodeGen/SPIRV/pointers/tangled-ret.ll

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
; The only pass criterion is that spirv-val considers output valid.
2+
3+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
4+
5+
%subgr = type { i64, i64 }
6+
%t_range = type { %t_arr }
7+
%t_arr = type { [1 x i64] }
8+
%t_arr2 = type { [4 x i32] }
9+
10+
define internal spir_func noundef i32 @geti32() {
11+
entry:
12+
ret i32 100
13+
}
14+
15+
define internal spir_func noundef i64 @geti64() {
16+
entry:
17+
ret i64 200
18+
}
19+
20+
define internal spir_func void @enable_if(ptr addrspace(4) noundef align 8 dereferenceable_or_null(8) %this, i64 noundef %dim0) {
21+
entry:
22+
%this.addr = alloca ptr addrspace(4), align 8
23+
%dim0.addr = alloca i64, align 8
24+
store ptr addrspace(4) %this, ptr %this.addr, align 8
25+
store i64 %dim0, ptr %dim0.addr, align 8
26+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
27+
%0 = load i64, ptr %dim0.addr, align 8
28+
call spir_func void @enable_if_2(ptr addrspace(4) noundef align 8 dereferenceable_or_null(8) %this1, i64 noundef %0)
29+
ret void
30+
}
31+
32+
33+
define internal spir_func void @test(ptr addrspace(4) noundef align 8 dereferenceable_or_null(16) %this, ptr addrspace(4) noundef align 4 dereferenceable(16) %bits, ptr noundef byval(%t_range) align 8 %pos) {
34+
entry:
35+
%this.addr = alloca ptr addrspace(4), align 8
36+
%bits.addr = alloca ptr addrspace(4), align 8
37+
%cur_pos = alloca i64, align 8
38+
%__range4 = alloca ptr addrspace(4), align 8
39+
%__begin0 = alloca ptr addrspace(4), align 8
40+
%__end0 = alloca ptr addrspace(4), align 8
41+
%cleanup.dest.slot = alloca i32, align 4
42+
%elem = alloca ptr addrspace(4), align 8
43+
%agg.tmp = alloca %t_range, align 8
44+
%agg.tmp.ascast = addrspacecast ptr %agg.tmp to ptr addrspace(4)
45+
store ptr addrspace(4) %this, ptr %this.addr, align 8
46+
store ptr addrspace(4) %bits, ptr %bits.addr, align 8
47+
%pos.ascast = addrspacecast ptr %pos to ptr addrspace(4)
48+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
49+
%call = call spir_func noundef i64 @getp(ptr addrspace(4) noundef align 8 dereferenceable_or_null(8) %pos.ascast, i32 noundef 0)
50+
store i64 %call, ptr %cur_pos, align 8
51+
%0 = load ptr addrspace(4), ptr %bits.addr, align 8
52+
store ptr addrspace(4) %0, ptr %__range4, align 8
53+
%1 = load ptr addrspace(4), ptr %__range4, align 8
54+
%call2 = call spir_func noundef ptr addrspace(4) @beginp(ptr addrspace(4) noundef align 4 dereferenceable_or_null(16) %1)
55+
store ptr addrspace(4) %call2, ptr %__begin0, align 8
56+
%2 = load ptr addrspace(4), ptr %__range4, align 8
57+
%call3 = call spir_func noundef ptr addrspace(4) @endp(ptr addrspace(4) noundef align 4 dereferenceable_or_null(16) %2)
58+
store ptr addrspace(4) %call3, ptr %__end0, align 8
59+
br label %for.cond
60+
61+
for.cond: ; preds = %for.inc, %entry
62+
%3 = load ptr addrspace(4), ptr %__begin0, align 8
63+
%4 = load ptr addrspace(4), ptr %__end0, align 8
64+
%cmp = icmp ne ptr addrspace(4) %3, %4
65+
br i1 %cmp, label %for.body, label %for.cond.cleanup
66+
67+
for.cond.cleanup: ; preds = %for.cond
68+
br label %for.end
69+
70+
for.body: ; preds = %for.cond
71+
%5 = load ptr addrspace(4), ptr %__begin0, align 8
72+
store ptr addrspace(4) %5, ptr %elem, align 8
73+
%6 = load i64, ptr %cur_pos, align 8
74+
%call4 = call spir_func noundef i32 @maskp(ptr addrspace(4) noundef align 8 dereferenceable_or_null(16) %this1)
75+
%conv = zext i32 %call4 to i64
76+
%cmp5 = icmp ult i64 %6, %conv
77+
br i1 %cmp5, label %if.then, label %if.else
78+
79+
if.then: ; preds = %for.body
80+
%7 = load ptr addrspace(4), ptr %elem, align 8
81+
%8 = load i64, ptr %cur_pos, align 8
82+
call spir_func void @enable_if(ptr addrspace(4) noundef align 8 dereferenceable_or_null(8) %agg.tmp.ascast, i64 noundef %8)
83+
call spir_func void @extract_bits(ptr addrspace(4) noundef align 8 dereferenceable_or_null(16) %this1, ptr addrspace(4) noundef align 4 dereferenceable(4) %7, ptr noundef byval(%t_range) align 8 %agg.tmp)
84+
%9 = load i64, ptr %cur_pos, align 8
85+
%add = add i64 %9, 32
86+
store i64 %add, ptr %cur_pos, align 8
87+
br label %if.end
88+
89+
if.else: ; preds = %for.body
90+
%10 = load ptr addrspace(4), ptr %elem, align 8
91+
store i32 0, ptr addrspace(4) %10, align 4
92+
br label %if.end
93+
94+
if.end: ; preds = %if.else, %if.then
95+
br label %for.inc
96+
97+
for.inc: ; preds = %if.end
98+
%11 = load ptr addrspace(4), ptr %__begin0, align 8
99+
%incdec.ptr = getelementptr inbounds nuw i32, ptr addrspace(4) %11, i32 1
100+
store ptr addrspace(4) %incdec.ptr, ptr %__begin0, align 8
101+
br label %for.cond
102+
103+
for.end: ; preds = %for.cond.cleanup
104+
ret void
105+
}
106+
107+
define internal spir_func noundef i64 @getp(ptr addrspace(4) noundef align 8 dereferenceable_or_null(8) %this, i32 noundef %dimension) {
108+
entry:
109+
%this.addr.i = alloca ptr addrspace(4), align 8
110+
%dimension.addr.i = alloca i32, align 4
111+
%retval = alloca i64, align 8
112+
%this.addr = alloca ptr addrspace(4), align 8
113+
%dimension.addr = alloca i32, align 4
114+
%retval.ascast = addrspacecast ptr %retval to ptr addrspace(4)
115+
store ptr addrspace(4) %this, ptr %this.addr, align 8
116+
store i32 %dimension, ptr %dimension.addr, align 4
117+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
118+
%0 = load i32, ptr %dimension.addr, align 4
119+
store ptr addrspace(4) %this1, ptr %this.addr.i, align 8
120+
store i32 %0, ptr %dimension.addr.i, align 4
121+
%this1.i = load ptr addrspace(4), ptr %this.addr.i, align 8
122+
%common_array1 = bitcast ptr addrspace(4) %this1 to ptr addrspace(4)
123+
%1 = load i32, ptr %dimension.addr, align 4
124+
%idxprom = sext i32 %1 to i64
125+
%arrayidx = getelementptr inbounds [1 x i64], ptr addrspace(4) %common_array1, i64 0, i64 %idxprom
126+
%2 = load i64, ptr addrspace(4) %arrayidx, align 8
127+
ret i64 %2
128+
}
129+
130+
define internal spir_func noundef ptr addrspace(4) @beginp(ptr addrspace(4) noundef align 4 dereferenceable_or_null(16) %this) {
131+
entry:
132+
%retval = alloca ptr addrspace(4), align 8
133+
%this.addr = alloca ptr addrspace(4), align 8
134+
%retval.ascast = addrspacecast ptr %retval to ptr addrspace(4)
135+
store ptr addrspace(4) %this, ptr %this.addr, align 8
136+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
137+
%MData1 = bitcast ptr addrspace(4) %this1 to ptr addrspace(4)
138+
%arraydecay2 = bitcast ptr addrspace(4) %MData1 to ptr addrspace(4)
139+
ret ptr addrspace(4) %arraydecay2
140+
}
141+
142+
define internal spir_func noundef ptr addrspace(4) @endp(ptr addrspace(4) noundef align 4 dereferenceable_or_null(16) %this) {
143+
entry:
144+
%retval = alloca ptr addrspace(4), align 8
145+
%this.addr = alloca ptr addrspace(4), align 8
146+
%retval.ascast = addrspacecast ptr %retval to ptr addrspace(4)
147+
store ptr addrspace(4) %this, ptr %this.addr, align 8
148+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
149+
%MData1 = bitcast ptr addrspace(4) %this1 to ptr addrspace(4)
150+
%arraydecay2 = bitcast ptr addrspace(4) %MData1 to ptr addrspace(4)
151+
%add.ptr = getelementptr inbounds nuw i32, ptr addrspace(4) %arraydecay2, i64 4
152+
ret ptr addrspace(4) %add.ptr
153+
}
154+
155+
define internal spir_func noundef i32 @maskp(ptr addrspace(4) noundef align 8 dereferenceable_or_null(16) %this) {
156+
entry:
157+
%retval = alloca i32, align 4
158+
%this.addr = alloca ptr addrspace(4), align 8
159+
%retval.ascast = addrspacecast ptr %retval to ptr addrspace(4)
160+
store ptr addrspace(4) %this, ptr %this.addr, align 8
161+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
162+
%bits_num = getelementptr inbounds nuw %subgr, ptr addrspace(4) %this1, i32 0, i32 1
163+
%0 = load i64, ptr addrspace(4) %bits_num, align 8
164+
%conv = trunc i64 %0 to i32
165+
ret i32 %conv
166+
}
167+
168+
define internal spir_func void @enable_if_2(ptr addrspace(4) noundef align 8 dereferenceable_or_null(8) %this, i64 noundef %dim0) {
169+
entry:
170+
%this.addr = alloca ptr addrspace(4), align 8
171+
%dim0.addr = alloca i64, align 8
172+
store ptr addrspace(4) %this, ptr %this.addr, align 8
173+
store i64 %dim0, ptr %dim0.addr, align 8
174+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
175+
%common_array1 = bitcast ptr addrspace(4) %this1 to ptr addrspace(4)
176+
%0 = load i64, ptr %dim0.addr, align 8
177+
store i64 %0, ptr addrspace(4) %common_array1, align 8
178+
ret void
179+
}
180+
181+
define internal spir_func void @extract_bits(ptr addrspace(4) noundef align 8 dereferenceable_or_null(16) %this, ptr addrspace(4) noundef align 4 dereferenceable(4) %bits, ptr noundef byval(%t_range) align 8 %pos) {
182+
entry:
183+
%this.addr = alloca ptr addrspace(4), align 8
184+
%bits.addr = alloca ptr addrspace(4), align 8
185+
%Res = alloca i64, align 8
186+
store ptr addrspace(4) %this, ptr %this.addr, align 8
187+
store ptr addrspace(4) %bits, ptr %bits.addr, align 8
188+
%pos.ascast = addrspacecast ptr %pos to ptr addrspace(4)
189+
%this1 = load ptr addrspace(4), ptr %this.addr, align 8
190+
%Bits1 = bitcast ptr addrspace(4) %this1 to ptr addrspace(4)
191+
%0 = load i64, ptr addrspace(4) %Bits1, align 8
192+
store i64 %0, ptr %Res, align 8
193+
%bits_num = getelementptr inbounds nuw %subgr, ptr addrspace(4) %this1, i32 0, i32 1
194+
%1 = load i64, ptr addrspace(4) %bits_num, align 8
195+
%call = call spir_func noundef i64 @geti64()
196+
%2 = load i64, ptr %Res, align 8
197+
%and = and i64 %2, %call
198+
store i64 %and, ptr %Res, align 8
199+
%call2 = call spir_func noundef i64 @geti64()
200+
%call3 = call spir_func noundef i32 @geti32()
201+
%conv = zext i32 %call3 to i64
202+
%cmp = icmp ult i64 %call2, %conv
203+
br i1 %cmp, label %if.then, label %if.else
204+
205+
if.else: ; preds = %entry
206+
%3 = load ptr addrspace(4), ptr %bits.addr, align 8
207+
store i32 0, ptr addrspace(4) %3, align 4
208+
br label %if.end11
209+
210+
if.then: ; preds = %entry
211+
%call4 = call spir_func noundef i64 @geti64()
212+
%cmp5 = icmp ugt i64 %call4, 0
213+
br i1 %cmp5, label %if.then6, label %if.end
214+
215+
if.then6: ; preds = %if.then
216+
%call7 = call spir_func noundef i64 @geti64()
217+
%4 = load i64, ptr %Res, align 8
218+
%shr = lshr i64 %4, %call7
219+
store i64 %shr, ptr %Res, align 8
220+
br label %if.end
221+
222+
if.end: ; preds = %if.then6, %if.then
223+
%call8 = call spir_func noundef i64 @geti64()
224+
%5 = load i64, ptr %Res, align 8
225+
%and9 = and i64 %5, %call8
226+
store i64 %and9, ptr %Res, align 8
227+
%6 = load i64, ptr %Res, align 8
228+
%conv10 = trunc i64 %6 to i32
229+
%7 = load ptr addrspace(4), ptr %bits.addr, align 8
230+
store i32 %conv10, ptr addrspace(4) %7, align 4
231+
br label %if.end11
232+
233+
if.end11: ; preds = %if.else, %if.end
234+
ret void
235+
}

0 commit comments

Comments
 (0)