Skip to content

Commit 95db1e9

Browse files
committed
[SPIR-V] Stop re-writing SPIR-V type map
There is a map used during forward translation. It links LLVM type and SPIR-V type, that is being generated, LLVM type here is used as a key. Due to a bug in type mapping (information is below), it's possible that for a single pointer type in LLVM, the translator will create 2 physically-indentical SPIR-V pointer types and with previous implementation of mapType function the map would be changed during the second type insertion. This patch prevents it, forcing to reuse the type that was created first, though we would still have an unused duplication of this type in SPIR-V module. Please consider following LLVM IR sample: %struct._ZTS4Args.Args = type { %struct._ZTS6Object.Object addrspace(4)* } %struct._ZTS6Object.Object = type { i32 (%struct._ZTS6Object.Object addrspace(4)*, i32)* } Here we have 2 LLVM type declarations. The translator will recursively go through structure members and then through element types of the pointer types, creating the appropriate SPIR-V types and mapping them on LLVM types. Due to a recursive nature of this algorithm, here we will have '%struct._ZTS6Object.Object addrspace(4)*' be processed twice with 2 SPIR-V type declaration instruction being created. Signed-off-by: Dmitry Sidorov <[email protected]>
1 parent 69fc6dc commit 95db1e9

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

llvm-spirv/lib/SPIRV/SPIRVWriter.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -1793,7 +1793,10 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
17931793
}
17941794

17951795
SPIRVType *LLVMToSPIRVBase::mapType(Type *T, SPIRVType *BT) {
1796-
TypeMap[T] = BT;
1796+
auto EmplaceStatus = TypeMap.try_emplace(T, BT);
1797+
(void)EmplaceStatus;
1798+
// TODO: Uncomment the assertion, once the type mapping issue is resolved
1799+
// assert(EmplaceStatus.second && "The type was already added to the map");
17971800
SPIRVDBG(dbgs() << "[mapType] " << *T << " => "; spvdbgs() << *BT << '\n');
17981801
return BT;
17991802
}
+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
; RUN: llvm-as %s -o %t.bc
2+
; RUN: llvm-spirv %t.bc --spirv-ext=+all -spirv-text -o %t
3+
; RUN: FileCheck < %t %s
4+
5+
; CHECK: Name [[#NAME:]] "struct._ZTS6Object.Object"
6+
; CHECK-COUNT-1: TypeStruct [[#NAME]]
7+
; TODO add check count one and remove unused, when the type mapping bug is fixed
8+
; CHECK: TypePointer [[#UNUSED:]] {{.*}} [[#NAME]]
9+
; CHECK: TypePointer [[#PTRTY:]] {{.*}} [[#NAME]]
10+
; CHECK-COUNT-2: TypePointer {{.*}} {{.*}} [[#PTRTY]]
11+
; CHECK-NOT: TypePointer {{.*}} {{.*}} [[#UNUSED]]
12+
13+
; ModuleID = 'sycl_test.bc'
14+
source_filename = "sycl_test.cpp"
15+
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
16+
target triple = "spir64-unknown-unknown-sycldevice"
17+
18+
%struct._ZTS4Args.Args = type { %struct._ZTS6Object.Object addrspace(4)* }
19+
%struct._ZTS6Object.Object = type { i32 (%struct._ZTS6Object.Object addrspace(4)*, i32)* }
20+
21+
; Function Attrs: convergent norecurse nounwind mustprogress
22+
define dso_local spir_func i32 @_Z9somefunc0P4Args(%struct._ZTS4Args.Args addrspace(4)* %args) #0 {
23+
entry:
24+
%retval = alloca i32, align 4
25+
%retval.ascast = addrspacecast i32* %retval to i32 addrspace(4)*
26+
%args.addr = alloca %struct._ZTS4Args.Args addrspace(4)*, align 8
27+
%args.addr.ascast = addrspacecast %struct._ZTS4Args.Args addrspace(4)** %args.addr to %struct._ZTS4Args.Args addrspace(4)* addrspace(4)*
28+
store %struct._ZTS4Args.Args addrspace(4)* %args, %struct._ZTS4Args.Args addrspace(4)* addrspace(4)* %args.addr.ascast, align 8, !tbaa !5
29+
ret i32 0
30+
}
31+
32+
; Function Attrs: convergent norecurse nounwind mustprogress
33+
define dso_local spir_func i32 @_Z9somefunc1P6Objecti(%struct._ZTS6Object.Object addrspace(4)* %object, i32 %value) #0 {
34+
entry:
35+
%retval = alloca i32, align 4
36+
%retval.ascast = addrspacecast i32* %retval to i32 addrspace(4)*
37+
%object.addr = alloca %struct._ZTS6Object.Object addrspace(4)*, align 8
38+
%object.addr.ascast = addrspacecast %struct._ZTS6Object.Object addrspace(4)** %object.addr to %struct._ZTS6Object.Object addrspace(4)* addrspace(4)*
39+
%value.addr = alloca i32, align 4
40+
%value.addr.ascast = addrspacecast i32* %value.addr to i32 addrspace(4)*
41+
store %struct._ZTS6Object.Object addrspace(4)* %object, %struct._ZTS6Object.Object addrspace(4)* addrspace(4)* %object.addr.ascast, align 8, !tbaa !5
42+
store i32 %value, i32 addrspace(4)* %value.addr.ascast, align 4, !tbaa !9
43+
ret i32 0
44+
}
45+
46+
attributes #0 = { convergent norecurse nounwind mustprogress "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "referenced-indirectly" "stack-protector-buffer-size"="8" "sycl-module-id"="sycl_test.cpp" }
47+
48+
!llvm.module.flags = !{!0, !1}
49+
!opencl.spir.version = !{!2}
50+
!spirv.Source = !{!3}
51+
!llvm.ident = !{!4}
52+
53+
!0 = !{i32 1, !"wchar_size", i32 4}
54+
!1 = !{i32 7, !"frame-pointer", i32 2}
55+
!2 = !{i32 1, i32 2}
56+
!3 = !{i32 4, i32 100000}
57+
!4 = !{!"clang version 13.0.0"}
58+
!5 = !{!6, !6, i64 0}
59+
!6 = !{!"any pointer", !7, i64 0}
60+
!7 = !{!"omnipotent char", !8, i64 0}
61+
!8 = !{!"Simple C++ TBAA"}
62+
!9 = !{!10, !10, i64 0}
63+
!10 = !{!"int", !7, i64 0}

0 commit comments

Comments
 (0)