Skip to content

Commit 0cf86c9

Browse files
[Backport to 19] Fix incorrect translation of calls to a builtin that returns a structure (#3027)
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 7902ffa commit 0cf86c9

File tree

2 files changed

+64
-5
lines changed

2 files changed

+64
-5
lines changed

lib/SPIRV/SPIRVUtil.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2227,8 +2227,9 @@ bool postProcessBuiltinReturningStruct(Function *F) {
22272227
Builder.SetInsertPoint(CI);
22282228
SmallVector<User *> Users(CI->users());
22292229
Value *A = nullptr;
2230+
StoreInst *SI = nullptr;
22302231
for (auto *U : Users) {
2231-
if (auto *SI = dyn_cast<StoreInst>(U)) {
2232+
if ((SI = dyn_cast<StoreInst>(U)) != nullptr) {
22322233
A = SI->getPointerOperand();
22332234
InstToRemove.push_back(SI);
22342235
break;
@@ -2248,12 +2249,18 @@ bool postProcessBuiltinReturningStruct(Function *F) {
22482249
NewF->setCallingConv(F->getCallingConv());
22492250
auto Args = getArguments(CI);
22502251
Args.insert(Args.begin(), A);
2251-
CallInst *NewCI = Builder.CreateCall(NewF, Args, CI->getName());
2252+
CallInst *NewCI = Builder.CreateCall(
2253+
NewF, Args, NewF->getReturnType()->isVoidTy() ? "" : CI->getName());
22522254
NewCI->addParamAttr(0, SretAttr);
22532255
NewCI->setCallingConv(CI->getCallingConv());
2254-
SmallVector<User *> CIUsers(CI->users());
2255-
for (auto *CIUser : CIUsers) {
2256-
CIUser->replaceUsesOfWith(CI, A);
2256+
SmallVector<User *, 32> UsersToReplace;
2257+
for (auto *U : Users)
2258+
if (U != SI)
2259+
UsersToReplace.push_back(U);
2260+
if (UsersToReplace.size() > 0) {
2261+
auto *LI = Builder.CreateLoad(F->getReturnType(), A);
2262+
for (auto *U : UsersToReplace)
2263+
U->replaceUsesOfWith(CI, LI);
22572264
}
22582265
InstToRemove.push_back(CI);
22592266
}

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(ptr sret(%structtype) %[[#Var:]]
51+
; CHECK: %[[#Load:]] = load %structtype, ptr %[[#Var]], align 4
52+
; CHECK-2: extractvalue %structtype %[[#Load:]]

0 commit comments

Comments
 (0)