Skip to content

Commit fc08bfe

Browse files
[Backport to 14] Fix incorrect translation of calls to a builtin that returns a structure (#2722) (#3032)
This PR fixes the issue with Khronos Translator incorrectly translating calls to builtins that returns a structure and generating incorrect extractvalue applied to a pointer. The PR is to fix the issue by preserving equivalence of newly inserted values with prior values of function return's type.
1 parent f59f24e commit fc08bfe

File tree

2 files changed

+71
-11
lines changed

2 files changed

+71
-11
lines changed

lib/SPIRV/SPIRVUtil.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ PointerType *getSPIRVOpaquePtrType(Module *M, Op OC) {
251251
}
252252

253253
void getFunctionTypeParameterTypes(llvm::FunctionType *FT,
254-
std::vector<Type *> &ArgTys) {
254+
SmallVector<Type *> &ArgTys) {
255255
for (auto I = FT->param_begin(), E = FT->param_end(); I != E; ++I) {
256256
ArgTys.push_back(*I);
257257
}
@@ -2072,10 +2072,11 @@ bool postProcessBuiltinReturningStruct(Function *F) {
20722072
if (auto *CI = dyn_cast<CallInst>(U)) {
20732073
IRBuilder<> Builder(CI->getParent());
20742074
Builder.SetInsertPoint(CI);
2075-
SmallVector<User *, 5> Users(CI->users());
2075+
SmallVector<User *> Users(CI->users());
20762076
Value *A = nullptr;
2077+
StoreInst *SI = nullptr;
20772078
for (auto *U : Users) {
2078-
if (auto *SI = dyn_cast<StoreInst>(U)) {
2079+
if ((SI = dyn_cast<StoreInst>(U)) != nullptr) {
20792080
A = SI->getPointerOperand();
20802081
InstToRemove.push_back(SI);
20812082
break;
@@ -2084,22 +2085,29 @@ bool postProcessBuiltinReturningStruct(Function *F) {
20842085
if (!A) {
20852086
A = Builder.CreateAlloca(F->getReturnType());
20862087
}
2087-
std::vector<Type *> ArgTys;
2088+
SmallVector<Type *> ArgTys;
20882089
getFunctionTypeParameterTypes(F->getFunctionType(), ArgTys);
20892090
ArgTys.insert(ArgTys.begin(), A->getType());
20902091
auto *NewF =
20912092
getOrCreateFunction(M, Type::getVoidTy(*Context), ArgTys, Name);
2092-
NewF->addParamAttr(0, Attribute::get(*Context,
2093-
Attribute::AttrKind::StructRet,
2094-
F->getReturnType()));
2093+
auto SretAttr = Attribute::get(*Context, Attribute::AttrKind::StructRet,
2094+
F->getReturnType());
2095+
NewF->addParamAttr(0, SretAttr);
20952096
NewF->setCallingConv(F->getCallingConv());
20962097
auto Args = getArguments(CI);
20972098
Args.insert(Args.begin(), A);
2098-
CallInst *NewCI = Builder.CreateCall(NewF, Args, CI->getName());
2099+
CallInst *NewCI = Builder.CreateCall(
2100+
NewF, Args, NewF->getReturnType()->isVoidTy() ? "" : CI->getName());
2101+
NewCI->addParamAttr(0, SretAttr);
20992102
NewCI->setCallingConv(CI->getCallingConv());
2100-
SmallVector<User *, 5> CIUsers(CI->users());
2101-
for (auto *CIUser : CIUsers) {
2102-
CIUser->replaceUsesOfWith(CI, A);
2103+
SmallVector<User *, 32> UsersToReplace;
2104+
for (auto *U : Users)
2105+
if (U != SI)
2106+
UsersToReplace.push_back(U);
2107+
if (UsersToReplace.size() > 0) {
2108+
auto *LI = Builder.CreateLoad(F->getReturnType(), A);
2109+
for (auto *U : UsersToReplace)
2110+
U->replaceUsesOfWith(CI, LI);
21032111
}
21042112
InstToRemove.push_back(CI);
21052113
}

test/builtin_returns_struct.spvasm

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
; REQUIRES: spirv-as
2+
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
3+
; RUN: spirv-val %t.spv
4+
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
5+
6+
OpCapability Kernel
7+
OpCapability Addresses
8+
OpCapability Int8
9+
OpCapability GenericPointer
10+
OpCapability Linkage
11+
%1 = OpExtInstImport "OpenCL.std"
12+
OpMemoryModel Physical64 OpenCL
13+
OpSource OpenCL_CPP 100000
14+
OpName %a "a"
15+
OpName %p "p"
16+
OpName %foo "foo"
17+
OpName %e "e"
18+
OpName %math "math"
19+
OpName %ov "ov"
20+
OpDecorate %foo LinkageAttributes "foo" Export
21+
%uint = OpTypeInt 32 0
22+
%uchar = OpTypeInt 8 0
23+
%_ptr_Generic_uchar = OpTypePointer Generic %uchar
24+
%5 = OpTypeFunction %uint %uint %_ptr_Generic_uchar
25+
%bool = OpTypeBool
26+
%_struct_7 = OpTypeStruct %uint %uint
27+
%uint_1 = OpConstant %uint 1
28+
%uchar_42 = OpConstant %uchar 42
29+
%10 = OpConstantNull %uint
30+
%foo = OpFunction %uint None %5
31+
%a = OpFunctionParameter %uint
32+
%p = OpFunctionParameter %_ptr_Generic_uchar
33+
%19 = OpLabel
34+
OpBranch %20
35+
%20 = OpLabel
36+
%e = OpPhi %uint %a %19 %math %21
37+
%16 = OpIAddCarry %_struct_7 %e %uint_1
38+
%math = OpCompositeExtract %uint %16 0
39+
%17 = OpCompositeExtract %uint %16 1
40+
%ov = OpINotEqual %bool %17 %10
41+
OpBranchConditional %ov %22 %21
42+
%21 = OpLabel
43+
OpStore %p %uchar_42 Aligned 1
44+
OpBranch %20
45+
%22 = OpLabel
46+
OpReturnValue %math
47+
OpFunctionEnd
48+
49+
; CHECK: %[[#Var:]] = alloca %structtype, align 8
50+
; CHECK: call spir_func void @_Z17__spirv_IAddCarryii(%structtype* sret(%structtype) %[[#Var:]]
51+
; CHECK: %[[#Load:]] = load %structtype, %structtype* %[[#Var]], align 4
52+
; CHECK-2: extractvalue %structtype %[[#Load:]]

0 commit comments

Comments
 (0)