Skip to content

Commit 7a45c84

Browse files
bokrzesiigcbot
authored andcommitted
Fixing ArgSize calculation for metadata type function argument in OpenCLPasses -> PrivateMemoryResolution and fix of invalid "sret" LegalizeFunctionSignatures void to struct transformation
In such case ``` ... call void @llvm.experimental.noalias.scope.decl(metadata !805) ... ; Function Attrs: nounwind declare spir_func %structtype @__devicelib_catanf(%structtype sret(%structtype) align 4 %0) #2 ; Function Attrs: nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite) declare void @llvm.experimental.noalias.scope.decl(metadata %0) #3 !805 = !{!806} !806 = distinct !{!806, !807, !"catanf: argument 0"} !807 = distinct !{!807, !"catanf"} ``` When PrivateMemoryResolution was "recursively calculating the max private mem usage of all callees" Then during processing function argument that is MetadataTy it was failing due to ``` Assertion failed: Ty->isSized() && "Cannot getTypeInfo() on a type that is unsized!", file ...\dump64\llvm-deps-16.0.6\src\llvm\lib\IR\DataLayout.cpp, line 751 ```` So I added check for MetadataTy which skips it. Additionally `LegalizeFunctionSignatures` was transforming the return type of void function that used sret() as its argument. The LLVM IR verifier didnt like that since the docs state that: `A function that accepts an sret argument must return void. A return value may not be sret.` I've decided to perform skip on sret cases in that pass.
1 parent e4c12ad commit 7a45c84

File tree

3 files changed

+28
-56
lines changed

3 files changed

+28
-56
lines changed

IGC/AdaptorCommon/LegalizeFunctionSignatures.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,18 @@ void LegalizeFunctionSignatures::FixFunctionSignatures(Module& M)
294294
// Create the new function signature by replacing the illegal types
295295
if (FunctionHasPromotableSRetArg(M, pFunc))
296296
{
297+
// According to docs: https://llvm.org/docs/LangRef.html
298+
// "A function that accepts an sret argument must return void. A return value may not be sret."
299+
// So I'm skipping further modifications of such functions
300+
// Because e.g
301+
// spir_func void @__devicelib_catanf(%structtype addrspace(4)* sret(%structtype) align 4, (...)
302+
// Was changed into
303+
// declare spir_func %structtype @__devicelib_catanf(%structtype sret(%structtype) align 4)
304+
// Which resulted in:
305+
// Invalid struct return type!
306+
// % structtype(% structtype)* @__devicelib_catanf
307+
continue;
308+
297309
retTypeOption = ReturnOpt::RETURN_STRUCT;
298310
ai++; // Skip adding the first arg
299311
}

IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/PrivateMemoryResolution.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,12 @@ bool PrivateMemoryResolution::runOnModule(llvm::Module& M)
374374
for (auto AI = childF->arg_begin(), AE = childF->arg_end(); AI != AE; ++AI)
375375
{
376376
// Argument offsets are also OWORD aligned
377-
argSize += iSTD::Align(static_cast<DWORD>(DL.getTypeAllocSize(AI->getType())), SIZE_OWORD);
377+
auto aiTy = AI->getType();
378+
379+
if (aiTy->isMetadataTy())
380+
continue;
381+
382+
argSize += iSTD::Align(static_cast<DWORD>(DL.getTypeAllocSize(aiTy)), SIZE_OWORD);
378383
}
379384
// Also do it for return value
380385
if (!childF->getReturnType()->isVoidTy())

IGC/Compiler/tests/LegalizeFunctionSignatures/fix-struct-ret-args.ll

Lines changed: 10 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
; ------------------------------------------------
1313

1414
; Pseudo-code:
15-
; Before:
1615
; void foo()
1716
; {
1817
; st_foo a;
@@ -29,81 +28,37 @@
2928
; {
3029
; *out = {1, 2};
3130
; }
32-
; After:
33-
; void foo()
34-
; {
35-
; st_foo a;
36-
; a = bar_0();
37-
; consume(&a);
38-
; }
39-
;
40-
; st_foo bar_0()
41-
; {
42-
; return bar_1();
43-
; }
44-
;
45-
; st_foo bar_1(st_foo* in)
46-
; {
47-
; st_foo out = {1, 2};
48-
; return out;
49-
; }
5031

5132
%struct._st_foo = type { i32, i32 }
5233

5334
define spir_kernel void @foo() #1 {
5435
; CHECK: define spir_kernel void @foo()
55-
%a = alloca %struct._st_foo
5636
; CHECK-NEXT: [[A:%.*]] = alloca %struct._st_foo, align 8
37+
; CHECK-NEXT: call void @bar_0(ptr sret(%struct._st_foo) [[A]])
38+
; CHECK-NEXT: call void @consume(ptr [[A]])
39+
; CHECK-NEXT: ret void
40+
%a = alloca %struct._st_foo
5741
call void @bar_0(ptr sret(%struct._st_foo) %a)
58-
; CHECK-NEXT: [[RET_BAR_0:%.*]] = call %struct._st_foo @bar_0()
59-
; CHECK-NEXT: [[M_0_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[A]], i32 0, i32 0
60-
; CHECK-NEXT: [[M_0:%.*]] = extractvalue %struct._st_foo [[RET_BAR_0]], 0
61-
; CHECK-NEXT: store i32 [[M_0]], ptr [[M_0_PTR]], align 4
62-
; CHECK-NEXT: [[M_1_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[A]], i32 0, i32 1
63-
; CHECK-NEXT: [[M_1:%.*]] = extractvalue %struct._st_foo [[RET_BAR_0]], 1
64-
; CHECK-NEXT: store i32 [[M_1]], ptr [[M_1_PTR]], align 4
6542
call void @consume(ptr %a)
66-
; CHECK-NEXT: call void @consume(ptr [[A]])
6743
ret void
68-
; CHECK-NEXT: ret void
6944
}
7045
; CHECK-NEXT: }
7146

7247
define linkonce_odr spir_func void @bar_0(ptr sret(%struct._st_foo) %out) {
73-
; CHECK: define linkonce_odr spir_func %struct._st_foo @bar_0()
74-
; CHECK-NEXT: [[OUT:%.*]] = alloca %struct._st_foo, align 8
48+
; CHECK: define linkonce_odr spir_func void @bar_0(ptr sret(%struct._st_foo) %out) {
49+
; CHECK-NEXT: call void @bar_1(ptr sret(%struct._st_foo) %out)
50+
; CHECK-NEXT: ret void
7551
call void @bar_1(ptr sret(%struct._st_foo) %out)
76-
; CHECK-NEXT: [[RET_BAR_1:%.*]] = call %struct._st_foo @bar_1()
77-
; CHECK-NEXT: [[M_0_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[OUT]], i32 0, i32 0
78-
; CHECK-NEXT: [[M_0:%.*]] = extractvalue %struct._st_foo [[RET_BAR_1]], 0
79-
; CHECK-NEXT: store i32 [[M_0]], ptr [[M_0_PTR]], align 4
80-
; CHECK-NEXT: [[M_1_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[OUT]], i32 0, i32 1
81-
; CHECK-NEXT: [[M_1:%.*]] = extractvalue %struct._st_foo [[RET_BAR_1]], 1
82-
; CHECK-NEXT: store i32 [[M_1]], ptr [[M_1_PTR]], align 4
8352
ret void
84-
; CHECK-NEXT: [[M_0_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[OUT]], i32 0, i32 0
85-
; CHECK-NEXT: [[M_0:%.*]] = load i32, ptr [[M_0_PTR]], align 4
86-
; CHECK-NEXT: [[R_0:%.*]] = insertvalue %struct._st_foo undef, i32 [[M_0]], 0
87-
; CHECK-NEXT: [[M_1_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[OUT]], i32 0, i32 1
88-
; CHECK-NEXT: [[M_1:%.*]] = load i32, ptr [[M_1_PTR]], align 4
89-
; CHECK-NEXT: [[R_1:%.*]] = insertvalue %struct._st_foo [[R_0]], i32 [[M_1]], 1
90-
; CHECK-NEXT: ret %struct._st_foo [[R_1]]
9153
}
9254
; CHECK-NEXT: }
9355

9456
define linkonce_odr spir_func void @bar_1(ptr sret(%struct._st_foo) %out) {
95-
; CHECK: define linkonce_odr spir_func %struct._st_foo @bar_1()
96-
; CHECK-NEXT: [[OUT:%.*]] = alloca %struct._st_foo, align 8
57+
; CHECK: define linkonce_odr spir_func void @bar_1(ptr sret(%struct._st_foo) %out) {
58+
; CHECK-NEXT: store %struct._st_foo { i32 1, i32 2 }, ptr %out, align 4
59+
; CHECK-NEXT: ret void
9760
store %struct._st_foo { i32 1, i32 2 }, ptr %out, align 4
98-
; CHECK-NEXT: store %struct._st_foo { i32 1, i32 2 }, ptr [[OUT]], align 4
9961
ret void
100-
; CHECK-NEXT: [[M_0_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[OUT]], i32 0, i32 0
101-
; CHECK-NEXT: [[M_0:%.*]] = load i32, ptr [[M_0_PTR]], align 4
102-
; CHECK-NEXT: [[R_0:%.*]] = insertvalue %struct._st_foo undef, i32 [[M_0]], 0
103-
; CHECK-NEXT: [[M_1_PTR:%.*]] = getelementptr inbounds %struct._st_foo, ptr [[OUT]], i32 0, i32 1
104-
; CHECK-NEXT: [[M_1:%.*]] = load i32, ptr [[M_1_PTR]], align 4
105-
; CHECK-NEXT: [[R_1:%.*]] = insertvalue %struct._st_foo [[R_0]], i32 [[M_1]], 1
106-
; CHECK-NEXT: ret %struct._st_foo [[R_1]]
10762
}
10863
; CHECK-NEXT: }
10964

0 commit comments

Comments
 (0)