Skip to content

Commit 47e996d

Browse files
VyacheslavLevytskyyVyacheslavLevytskyy
authored
[SPIR-V] Fix OpVariable instructions place in a function (llvm#87554)
This PR: * fixes OpVariable instructions place in a function (see llvm#66261), * improves type inference, * helps avoiding unneeded bitcasts when validating function call's This allows to improve existing and add new test cases with more strict checks. OpVariable fix refers to "All OpVariable instructions in a function must be the first instructions in the first block" requirement from SPIR-V spec.
1 parent a2306b6 commit 47e996d

File tree

7 files changed

+122
-13
lines changed

7 files changed

+122
-13
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,12 @@ static SPIRVType *getArgSPIRVType(const Function &F, unsigned ArgIdx,
243243
continue;
244244

245245
MetadataAsValue *VMD = cast<MetadataAsValue>(II->getOperand(1));
246-
SPIRVType *ElementType = GR->getOrCreateSPIRVType(
247-
cast<ConstantAsMetadata>(VMD->getMetadata())->getType(), MIRBuilder);
246+
Type *ElementTy = cast<ConstantAsMetadata>(VMD->getMetadata())->getType();
247+
if (isUntypedPointerTy(ElementTy))
248+
ElementTy =
249+
TypedPointerType::get(IntegerType::getInt8Ty(II->getContext()),
250+
getPointerAddressSpace(ElementTy));
251+
SPIRVType *ElementType = GR->getOrCreateSPIRVType(ElementTy, MIRBuilder);
248252
return GR->getOrCreateSPIRVPointerType(
249253
ElementType, MIRBuilder,
250254
addressSpaceToStorageClass(

‎llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class SPIRVGlobalRegistry {
4747
DenseMap<const MachineOperand *, const Function *> InstrToFunction;
4848
// Maps Functions to their calls (in a form of the machine instruction,
4949
// OpFunctionCall) that happened before the definition is available
50-
DenseMap<const Function *, SmallVector<MachineInstr *>> ForwardCalls;
50+
DenseMap<const Function *, SmallPtrSet<MachineInstr *, 8>> ForwardCalls;
5151

5252
// Look for an equivalent of the newType in the map. Return the equivalent
5353
// if it's found, otherwise insert newType to the map and return the type.
@@ -215,12 +215,12 @@ class SPIRVGlobalRegistry {
215215
if (It == ForwardCalls.end())
216216
ForwardCalls[F] = {MI};
217217
else
218-
It->second.push_back(MI);
218+
It->second.insert(MI);
219219
}
220220

221221
// Map a Function to the vector of machine instructions that represents
222222
// forward function calls or to nullptr if not found.
223-
SmallVector<MachineInstr *> *getForwardCalls(const Function *F) {
223+
SmallPtrSet<MachineInstr *, 8> *getForwardCalls(const Function *F) {
224224
auto It = ForwardCalls.find(F);
225225
return It == ForwardCalls.end() ? nullptr : &It->second;
226226
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ void validateForwardCalls(const SPIRVSubtarget &STI,
193193
MachineRegisterInfo *DefMRI, SPIRVGlobalRegistry &GR,
194194
MachineInstr &FunDef) {
195195
const Function *F = GR.getFunctionByDefinition(&FunDef);
196-
if (SmallVector<MachineInstr *> *FwdCalls = GR.getForwardCalls(F))
196+
if (SmallPtrSet<MachineInstr *, 8> *FwdCalls = GR.getForwardCalls(F))
197197
for (MachineInstr *FunCall : *FwdCalls) {
198198
MachineRegisterInfo *CallMRI =
199199
&FunCall->getParent()->getParent()->getRegInfo();

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1825,7 +1825,24 @@ bool SPIRVInstructionSelector::selectAllocaArray(Register ResVReg,
18251825
bool SPIRVInstructionSelector::selectFrameIndex(Register ResVReg,
18261826
const SPIRVType *ResType,
18271827
MachineInstr &I) const {
1828-
return BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpVariable))
1828+
// Change order of instructions if needed: all OpVariable instructions in a
1829+
// function must be the first instructions in the first block
1830+
MachineFunction *MF = I.getParent()->getParent();
1831+
MachineBasicBlock *MBB = &MF->front();
1832+
auto It = MBB->SkipPHIsAndLabels(MBB->begin()), E = MBB->end();
1833+
bool IsHeader = false;
1834+
unsigned Opcode;
1835+
for (; It != E && It != I; ++It) {
1836+
Opcode = It->getOpcode();
1837+
if (Opcode == SPIRV::OpFunction || Opcode == SPIRV::OpFunctionParameter) {
1838+
IsHeader = true;
1839+
} else if (IsHeader &&
1840+
!(Opcode == SPIRV::ASSIGN_TYPE || Opcode == SPIRV::OpLabel)) {
1841+
++It;
1842+
break;
1843+
}
1844+
}
1845+
return BuildMI(*MBB, It, It->getDebugLoc(), TII.get(SPIRV::OpVariable))
18291846
.addDef(ResVReg)
18301847
.addUse(GR.getSPIRVTypeID(ResType))
18311848
.addImm(static_cast<uint32_t>(SPIRV::StorageClass::Function))

‎llvm/test/CodeGen/SPIRV/OpVariable_order.ll

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
; REQUIRES: spirv-tools
2-
; RUN: llc -O0 -mtriple=spirv-unknown-linux %s -o - -filetype=obj | not spirv-val 2>&1 | FileCheck %s
1+
; All OpVariable instructions in a function must be the first instructions in the first block
32

4-
; TODO(#66261): The SPIR-V backend should reorder OpVariable instructions so this doesn't fail,
5-
; but in the meantime it's a good example of the spirv-val tool working as intended.
3+
; RUN: llc -O0 -mtriple=spirv-unknown-linux %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
4+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-linux %s -o - -filetype=obj | spirv-val %}
65

7-
; CHECK: All OpVariable instructions in a function must be the first instructions in the first block.
6+
; CHECK-SPIRV: OpFunction
7+
; CHECK-SPIRV-NEXT: OpLabel
8+
; CHECK-SPIRV-NEXT: OpVariable
9+
; CHECK-SPIRV-NEXT: OpVariable
10+
; CHECK-SPIRV: OpReturn
11+
; CHECK-SPIRV: OpFunctionEnd
812

913
define void @main() #1 {
1014
entry:

‎llvm/test/CodeGen/SPIRV/pointers/type-deduce-by-call-chain.ll

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,46 @@
1010
; CHECK-SPIRV-DAG: OpName %[[FooObj:.*]] "foo_object"
1111
; CHECK-SPIRV-DAG: OpName %[[FooMemOrder:.*]] "mem_order"
1212
; CHECK-SPIRV-DAG: OpName %[[FooFunc:.*]] "foo"
13+
1314
; CHECK-SPIRV-DAG: %[[TyLong:.*]] = OpTypeInt 32 0
1415
; CHECK-SPIRV-DAG: %[[TyVoid:.*]] = OpTypeVoid
16+
; CHECK-SPIRV-DAG: %[[TyGenPtrLong:.*]] = OpTypePointer Generic %[[TyLong]]
1517
; CHECK-SPIRV-DAG: %[[TyPtrLong:.*]] = OpTypePointer CrossWorkgroup %[[TyLong]]
1618
; CHECK-SPIRV-DAG: %[[TyFunPtrLong:.*]] = OpTypeFunction %[[TyVoid]] %[[TyPtrLong]]
17-
; CHECK-SPIRV-DAG: %[[TyGenPtrLong:.*]] = OpTypePointer Generic %[[TyLong]]
19+
; CHECK-SPIRV-DAG: %[[TyGenPtrPtrLong:.*]] = OpTypePointer Generic %[[TyGenPtrLong]]
1820
; CHECK-SPIRV-DAG: %[[TyFunGenPtrLongLong:.*]] = OpTypeFunction %[[TyVoid]] %[[TyGenPtrLong]] %[[TyLong]]
21+
; CHECK-SPIRV-DAG: %[[TyChar:.*]] = OpTypeInt 8 0
22+
; CHECK-SPIRV-DAG: %[[TyGenPtrChar:.*]] = OpTypePointer Generic %[[TyChar]]
23+
; CHECK-SPIRV-DAG: %[[TyGenPtrPtrChar:.*]] = OpTypePointer Generic %[[TyGenPtrChar]]
24+
; CHECK-SPIRV-DAG: %[[TyFunPtrGenPtrChar:.*]] = OpTypePointer Function %[[TyGenPtrChar]]
1925
; CHECK-SPIRV-DAG: %[[Const3:.*]] = OpConstant %[[TyLong]] 3
26+
2027
; CHECK-SPIRV: %[[FunTest]] = OpFunction %[[TyVoid]] None %[[TyFunPtrLong]]
2128
; CHECK-SPIRV: %[[ArgCum]] = OpFunctionParameter %[[TyPtrLong]]
29+
2230
; CHECK-SPIRV: OpFunctionCall %[[TyVoid]] %[[FooFunc]] %[[Addr]] %[[Const3]]
31+
32+
; CHECK-SPIRV: %[[HalfAddr:.*]] = OpPtrCastToGeneric
33+
; CHECK-SPIRV-NEXT: %[[HalfAddrCasted:.*]] = OpBitcast %[[TyGenPtrLong]] %[[HalfAddr]]
34+
; CHECK-SPIRV-NEXT: OpFunctionCall %[[TyVoid]] %[[FooFunc]] %[[HalfAddrCasted]] %[[Const3]]
35+
36+
; CHECK-SPIRV: %[[DblAddr:.*]] = OpPtrCastToGeneric
37+
; CHECK-SPIRV-NEXT: %[[DblAddrCasted:.*]] = OpBitcast %[[TyGenPtrLong]] %[[DblAddr]]
38+
; CHECK-SPIRV-NEXT: OpFunctionCall %[[TyVoid]] %[[FooFunc]] %[[DblAddrCasted]] %[[Const3]]
39+
2340
; CHECK-SPIRV: %[[FooStub]] = OpFunction %[[TyVoid]] None %[[TyFunGenPtrLongLong]]
2441
; CHECK-SPIRV: %[[StubObj]] = OpFunctionParameter %[[TyGenPtrLong]]
2542
; CHECK-SPIRV: %[[MemOrder]] = OpFunctionParameter %[[TyLong]]
43+
44+
; CHECK-SPIRV: %[[ObjectAddr:.*]] = OpVariable %[[TyFunPtrGenPtrChar]] Function
45+
; CHECK-SPIRV-NEXT: %[[ToGeneric:.*]] = OpPtrCastToGeneric %[[TyGenPtrPtrChar]] %[[ObjectAddr]]
46+
; CHECK-SPIRV-NEXT: %[[Casted:.*]] = OpBitcast %[[TyGenPtrPtrLong]] %[[ToGeneric]]
47+
; CHECK-SPIRV-NEXT: OpStore %[[Casted]] %[[StubObj]]
48+
2649
; CHECK-SPIRV: %[[FooFunc]] = OpFunction %[[TyVoid]] None %[[TyFunGenPtrLongLong]]
2750
; CHECK-SPIRV: %[[FooObj]] = OpFunctionParameter %[[TyGenPtrLong]]
2851
; CHECK-SPIRV: %[[FooMemOrder]] = OpFunctionParameter %[[TyLong]]
52+
2953
; CHECK-SPIRV: OpFunctionCall %[[TyVoid]] %[[FooStub]] %[[FooObj]] %[[FooMemOrder]]
3054

3155
define spir_kernel void @test(ptr addrspace(1) noundef align 4 %_arg_cum) {

‎llvm/test/CodeGen/SPIRV/pointers/type-deduce-call-no-bitcast.ll

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
2+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
3+
4+
; CHECK-SPIRV-DAG: OpName %[[Foo:.*]] "foo"
5+
; CHECK-SPIRV-DAG: %[[TyChar:.*]] = OpTypeInt 8 0
6+
; CHECK-SPIRV-DAG: %[[TyVoid:.*]] = OpTypeVoid
7+
; CHECK-SPIRV-DAG: %[[TyGenPtrChar:.*]] = OpTypePointer Generic %[[TyChar]]
8+
; CHECK-SPIRV-DAG: %[[TyFunBar:.*]] = OpTypeFunction %[[TyVoid]] %[[TyGenPtrChar]]
9+
; CHECK-SPIRV-DAG: %[[TyLong:.*]] = OpTypeInt 64 0
10+
; CHECK-SPIRV-DAG: %[[TyGenPtrPtrChar:.*]] = OpTypePointer Generic %[[TyGenPtrChar]]
11+
; CHECK-SPIRV-DAG: %[[TyFunFoo:.*]] = OpTypeFunction %[[TyVoid]] %[[TyLong]] %[[TyGenPtrPtrChar]] %[[TyGenPtrPtrChar]]
12+
; CHECK-SPIRV-DAG: %[[TyStruct:.*]] = OpTypeStruct %[[TyLong]]
13+
; CHECK-SPIRV-DAG: %[[Const100:.*]] = OpConstant %[[TyLong]] 100
14+
; CHECK-SPIRV-DAG: %[[TyFunPtrGenPtrChar:.*]] = OpTypePointer Function %[[TyGenPtrChar]]
15+
; CHECK-SPIRV-DAG: %[[TyPtrStruct:.*]] = OpTypePointer Generic %[[TyStruct]]
16+
; CHECK-SPIRV-DAG: %[[TyPtrLong:.*]] = OpTypePointer Generic %[[TyLong]]
17+
18+
; CHECK-SPIRV: %[[Bar:.*]] = OpFunction %[[TyVoid]] None %[[TyFunBar]]
19+
; CHECK-SPIRV: %[[BarArg:.*]] = OpFunctionParameter %[[TyGenPtrChar]]
20+
; CHECK-SPIRV-NEXT: OpLabel
21+
; CHECK-SPIRV-NEXT: OpVariable %[[TyFunPtrGenPtrChar]] Function
22+
; CHECK-SPIRV-NEXT: OpVariable %[[TyFunPtrGenPtrChar]] Function
23+
; CHECK-SPIRV-NEXT: OpVariable %[[TyFunPtrGenPtrChar]] Function
24+
; CHECK-SPIRV: %[[Var1:.*]] = OpPtrCastToGeneric %[[TyGenPtrPtrChar]] %[[#]]
25+
; CHECK-SPIRV: %[[Var2:.*]] = OpPtrCastToGeneric %[[TyGenPtrPtrChar]] %[[#]]
26+
; CHECK-SPIRV: OpStore %[[#]] %[[BarArg]]
27+
; CHECK-SPIRV-NEXT: OpFunctionCall %[[TyVoid]] %[[Foo]] %[[Const100]] %[[Var1]] %[[Var2]]
28+
; CHECK-SPIRV-NEXT: OpFunctionCall %[[TyVoid]] %[[Foo]] %[[Const100]] %[[Var2]] %[[Var1]]
29+
30+
; CHECK-SPIRV: %[[Foo]] = OpFunction %[[TyVoid]] None %[[TyFunFoo]]
31+
; CHECK-SPIRV-NEXT: OpFunctionParameter %[[TyLong]]
32+
; CHECK-SPIRV-NEXT: OpFunctionParameter %[[TyGenPtrPtrChar]]
33+
; CHECK-SPIRV-NEXT: OpFunctionParameter %[[TyGenPtrPtrChar]]
34+
35+
%class.CustomType = type { i64 }
36+
37+
define linkonce_odr dso_local spir_func void @bar(ptr addrspace(4) noundef %first) {
38+
entry:
39+
%first.addr = alloca ptr addrspace(4)
40+
%first.addr.ascast = addrspacecast ptr %first.addr to ptr addrspace(4)
41+
%temp = alloca ptr addrspace(4), align 8
42+
%temp.ascast = addrspacecast ptr %temp to ptr addrspace(4)
43+
store ptr addrspace(4) %first, ptr %first.addr
44+
call spir_func void @foo(i64 noundef 100, ptr addrspace(4) noundef dereferenceable(8) %first.addr.ascast, ptr addrspace(4) noundef dereferenceable(8) %temp.ascast)
45+
call spir_func void @foo(i64 noundef 100, ptr addrspace(4) noundef dereferenceable(8) %temp.ascast, ptr addrspace(4) noundef dereferenceable(8) %first.addr.ascast)
46+
%var = alloca ptr addrspace(4), align 8
47+
ret void
48+
}
49+
50+
define linkonce_odr dso_local spir_func void @foo(i64 noundef %offset, ptr addrspace(4) noundef dereferenceable(8) %in_acc1, ptr addrspace(4) noundef dereferenceable(8) %out_acc1) {
51+
entry:
52+
%r0 = load ptr addrspace(4), ptr addrspace(4) %in_acc1
53+
%arrayidx = getelementptr inbounds %class.CustomType, ptr addrspace(4) %r0, i64 42
54+
%r1 = load i64, ptr addrspace(4) %arrayidx
55+
%r3 = load ptr addrspace(4), ptr addrspace(4) %out_acc1
56+
%r4 = getelementptr %class.CustomType, ptr addrspace(4) %r3, i64 43
57+
store i64 %r1, ptr addrspace(4) %r4
58+
ret void
59+
}
60+

0 commit comments

Comments
 (0)