Skip to content

Commit 3ac1041

Browse files
VyacheslavLevytskyyVyacheslavLevytskyy
authored
[Backport to 18] Fix incorrect translation of calls to a builtin that returns a structure (#2722) (#3028)
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 bfd8955 commit 3ac1041

File tree

5 files changed

+110
-26
lines changed

5 files changed

+110
-26
lines changed

‎lib/SPIRV/SPIRVReader.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2758,20 +2758,12 @@ Value *SPIRVToLLVM::transValueWithoutDecoration(SPIRVValue *BV, Function *F,
27582758
return mapValue(BV,
27592759
transRelational(static_cast<SPIRVInstruction *>(BV), BB));
27602760
case OpIAddCarry: {
2761-
IRBuilder Builder(BB);
27622761
auto *BC = static_cast<SPIRVBinary *>(BV);
2763-
return mapValue(BV, Builder.CreateBinaryIntrinsic(
2764-
Intrinsic::uadd_with_overflow,
2765-
transValue(BC->getOperand(0), F, BB),
2766-
transValue(BC->getOperand(1), F, BB)));
2762+
return mapValue(BV, transBuiltinFromInst("__spirv_IAddCarry", BC, BB));
27672763
}
27682764
case OpISubBorrow: {
2769-
IRBuilder Builder(BB);
27702765
auto *BC = static_cast<SPIRVBinary *>(BV);
2771-
return mapValue(BV, Builder.CreateBinaryIntrinsic(
2772-
Intrinsic::usub_with_overflow,
2773-
transValue(BC->getOperand(0), F, BB),
2774-
transValue(BC->getOperand(1), F, BB)));
2766+
return mapValue(BV, transBuiltinFromInst("__spirv_ISubBorrow", BC, BB));
27752767
}
27762768
case OpGetKernelWorkGroupSize:
27772769
case OpGetKernelPreferredWorkGroupSizeMultiple:

‎lib/SPIRV/SPIRVUtil.cpp

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ StructType *getOrCreateOpaqueStructType(Module *M, StringRef Name) {
185185
}
186186

187187
void getFunctionTypeParameterTypes(llvm::FunctionType *FT,
188-
std::vector<Type *> &ArgTys) {
188+
SmallVector<Type *> &ArgTys) {
189189
for (auto I = FT->param_begin(), E = FT->param_end(); I != E; ++I) {
190190
ArgTys.push_back(*I);
191191
}
@@ -2188,23 +2188,45 @@ bool postProcessBuiltinReturningStruct(Function *F) {
21882188
SmallVector<Instruction *, 32> InstToRemove;
21892189
for (auto *U : F->users()) {
21902190
if (auto *CI = dyn_cast<CallInst>(U)) {
2191-
auto *ST = cast<StoreInst>(*(CI->user_begin()));
2192-
std::vector<Type *> ArgTys;
2191+
IRBuilder<> Builder(CI->getParent());
2192+
Builder.SetInsertPoint(CI);
2193+
SmallVector<User *> Users(CI->users());
2194+
Value *A = nullptr;
2195+
StoreInst *SI = nullptr;
2196+
for (auto *U : Users) {
2197+
if ((SI = dyn_cast<StoreInst>(U)) != nullptr) {
2198+
A = SI->getPointerOperand();
2199+
InstToRemove.push_back(SI);
2200+
break;
2201+
}
2202+
}
2203+
if (!A) {
2204+
A = Builder.CreateAlloca(F->getReturnType());
2205+
}
2206+
SmallVector<Type *> ArgTys;
21932207
getFunctionTypeParameterTypes(F->getFunctionType(), ArgTys);
2194-
ArgTys.insert(ArgTys.begin(),
2195-
PointerType::get(F->getReturnType(), SPIRAS_Private));
2208+
ArgTys.insert(ArgTys.begin(), A->getType());
21962209
auto *NewF =
21972210
getOrCreateFunction(M, Type::getVoidTy(*Context), ArgTys, Name);
21982211
auto SretAttr = Attribute::get(*Context, Attribute::AttrKind::StructRet,
21992212
F->getReturnType());
22002213
NewF->addParamAttr(0, SretAttr);
22012214
NewF->setCallingConv(F->getCallingConv());
22022215
auto Args = getArguments(CI);
2203-
Args.insert(Args.begin(), ST->getPointerOperand());
2204-
auto *NewCI = CallInst::Create(NewF, Args, CI->getName(), CI);
2216+
Args.insert(Args.begin(), A);
2217+
CallInst *NewCI = Builder.CreateCall(
2218+
NewF, Args, NewF->getReturnType()->isVoidTy() ? "" : CI->getName());
22052219
NewCI->addParamAttr(0, SretAttr);
22062220
NewCI->setCallingConv(CI->getCallingConv());
2207-
InstToRemove.push_back(ST);
2221+
SmallVector<User *, 32> UsersToReplace;
2222+
for (auto *U : Users)
2223+
if (U != SI)
2224+
UsersToReplace.push_back(U);
2225+
if (UsersToReplace.size() > 0) {
2226+
auto *LI = Builder.CreateLoad(F->getReturnType(), A);
2227+
for (auto *U : UsersToReplace)
2228+
U->replaceUsesOfWith(CI, LI);
2229+
}
22082230
InstToRemove.push_back(CI);
22092231
}
22102232
}

‎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:]]

‎test/llvm-intrinsics/uadd.with.overflow.ll

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,19 @@ target triple = "spir64-unknown-unknown"
2222
; CHECK-SPIRV: IAddCarry [[#S1TYPE]]
2323
; CHECK-SPIRV: IAddCarry [[#S2TYPE]]
2424
; CHECK-SPIRV: IAddCarry [[#S3TYPE]]
25-
; CHECK-LLVM: call { i16, i1 } @llvm.uadd.with.overflow.i16(i16 %a, i16 %b)
26-
; CHECK-LLVM: call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 %a, i32 %b)
27-
; CHECK-LLVM: call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 %a, i64 %b)
28-
; CHECK-LLVM: call { <4 x i32>, <4 x i1> } @llvm.uadd.with.overflow.v4i32(<4 x i32> %a, <4 x i32> %b)
25+
26+
; CHECK-LLVM: %structtype = type { i16, i1 }
27+
; CHECK-LLVM: %structtype.0 = type { i32, i1 }
28+
; CHECK-LLVM: %structtype.1 = type { i64, i1 }
29+
; CHECK-LLVM: %structtype.2 = type { <4 x i32>, <4 x i1> }
30+
; CHECK-LLVM: %0 = alloca %structtype, align 8
31+
; CHECK-LLVM: call spir_func void @_Z17__spirv_IAddCarryss(ptr sret(%structtype) %0, i16 %a, i16 %b)
32+
; CHECK-LLVM: %0 = alloca %structtype.0, align 8
33+
; CHECK-LLVM: call spir_func void @_Z17__spirv_IAddCarryii(ptr sret(%structtype.0) %0, i32 %a, i32 %b)
34+
; CHECK-LLVM: %0 = alloca %structtype.1, align 8
35+
; CHECK-LLVM: call spir_func void @_Z17__spirv_IAddCarryll(ptr sret(%structtype.1) %0, i64 %a, i64 %b)
36+
; CHECK-LLVM: %0 = alloca %structtype.2, align 16
37+
; CHECK-LLVM: call spir_func void @_Z17__spirv_IAddCarryDv4_iS_(ptr sret(%structtype.2) %0, <4 x i32> %a, <4 x i32> %b)
2938

3039
define spir_func void @test_uadd_with_overflow_i16(i16 %a, i16 %b) {
3140
entry:

‎test/llvm-intrinsics/usub.with.overflow.ll

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,19 @@ target triple = "spir64-unknown-unknown"
2323
; CHECK-SPIRV: ISubBorrow [[#S1TYPE]]
2424
; CHECK-SPIRV: ISubBorrow [[#S2TYPE]]
2525
; CHECK-SPIRV: ISubBorrow [[#S3TYPE]]
26-
; CHECK-LLVM: call { i16, i1 } @llvm.usub.with.overflow.i16(i16 %a, i16 %b)
27-
; CHECK-LLVM: call { i32, i1 } @llvm.usub.with.overflow.i32(i32 %a, i32 %b)
28-
; CHECK-LLVM: call { i64, i1 } @llvm.usub.with.overflow.i64(i64 %a, i64 %b)
29-
; CHECK-LLVM: call { <4 x i32>, <4 x i1> } @llvm.usub.with.overflow.v4i32(<4 x i32> %a, <4 x i32> %b)
26+
27+
; CHECK-LLVM: %structtype = type { i16, i1 }
28+
; CHECK-LLVM: %structtype.0 = type { i32, i1 }
29+
; CHECK-LLVM: %structtype.1 = type { i64, i1 }
30+
; CHECK-LLVM: %structtype.2 = type { <4 x i32>, <4 x i1> }
31+
; CHECK-LLVM: %0 = alloca %structtype, align 8
32+
; CHECK-LLVM: call spir_func void @_Z18__spirv_ISubBorrowss(ptr sret(%structtype) %0, i16 %a, i16 %b)
33+
; CHECK-LLVM: %0 = alloca %structtype.0, align 8
34+
; CHECK-LLVM: call spir_func void @_Z18__spirv_ISubBorrowii(ptr sret(%structtype.0) %0, i32 %a, i32 %b)
35+
; CHECK-LLVM: %0 = alloca %structtype.1, align 8
36+
; CHECK-LLVM: call spir_func void @_Z18__spirv_ISubBorrowll(ptr sret(%structtype.1) %0, i64 %a, i64 %b)
37+
; CHECK-LLVM: %0 = alloca %structtype.2, align 16
38+
; CHECK-LLVM: call spir_func void @_Z18__spirv_ISubBorrowDv4_iS_(ptr sret(%structtype.2) %0, <4 x i32> %a, <4 x i32> %b)
3039

3140
define spir_func void @test_usub_with_overflow_i16(i16 %a, i16 %b) {
3241
entry:

0 commit comments

Comments
 (0)