From 95db1e9ad3930cb7a12c9690ee9cea43c8706ea1 Mon Sep 17 00:00:00 2001 From: Dmitry Sidorov Date: Mon, 31 May 2021 20:44:45 +0300 Subject: [PATCH 1/2] [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 --- llvm-spirv/lib/SPIRV/SPIRVWriter.cpp | 5 +- llvm-spirv/test/pointer_type_mapping.ll | 63 +++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 llvm-spirv/test/pointer_type_mapping.ll diff --git a/llvm-spirv/lib/SPIRV/SPIRVWriter.cpp b/llvm-spirv/lib/SPIRV/SPIRVWriter.cpp index 2853410fd8b61..8071d6dfcdc5e 100644 --- a/llvm-spirv/lib/SPIRV/SPIRVWriter.cpp +++ b/llvm-spirv/lib/SPIRV/SPIRVWriter.cpp @@ -1793,7 +1793,10 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB, } SPIRVType *LLVMToSPIRVBase::mapType(Type *T, SPIRVType *BT) { - TypeMap[T] = BT; + auto EmplaceStatus = TypeMap.try_emplace(T, BT); + (void)EmplaceStatus; + // TODO: Uncomment the assertion, once the type mapping issue is resolved + // assert(EmplaceStatus.second && "The type was already added to the map"); SPIRVDBG(dbgs() << "[mapType] " << *T << " => "; spvdbgs() << *BT << '\n'); return BT; } diff --git a/llvm-spirv/test/pointer_type_mapping.ll b/llvm-spirv/test/pointer_type_mapping.ll new file mode 100644 index 0000000000000..7950cda7e529d --- /dev/null +++ b/llvm-spirv/test/pointer_type_mapping.ll @@ -0,0 +1,63 @@ +; RUN: llvm-as %s -o %t.bc +; RUN: llvm-spirv %t.bc --spirv-ext=+all -spirv-text -o %t +; RUN: FileCheck < %t %s + +; CHECK: Name [[#NAME:]] "struct._ZTS6Object.Object" +; CHECK-COUNT-1: TypeStruct [[#NAME]] +; TODO add check count one and remove unused, when the type mapping bug is fixed +; CHECK: TypePointer [[#UNUSED:]] {{.*}} [[#NAME]] +; CHECK: TypePointer [[#PTRTY:]] {{.*}} [[#NAME]] +; CHECK-COUNT-2: TypePointer {{.*}} {{.*}} [[#PTRTY]] +; CHECK-NOT: TypePointer {{.*}} {{.*}} [[#UNUSED]] + +; ModuleID = 'sycl_test.bc' +source_filename = "sycl_test.cpp" +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" +target triple = "spir64-unknown-unknown-sycldevice" + +%struct._ZTS4Args.Args = type { %struct._ZTS6Object.Object addrspace(4)* } +%struct._ZTS6Object.Object = type { i32 (%struct._ZTS6Object.Object addrspace(4)*, i32)* } + +; Function Attrs: convergent norecurse nounwind mustprogress +define dso_local spir_func i32 @_Z9somefunc0P4Args(%struct._ZTS4Args.Args addrspace(4)* %args) #0 { +entry: + %retval = alloca i32, align 4 + %retval.ascast = addrspacecast i32* %retval to i32 addrspace(4)* + %args.addr = alloca %struct._ZTS4Args.Args addrspace(4)*, align 8 + %args.addr.ascast = addrspacecast %struct._ZTS4Args.Args addrspace(4)** %args.addr to %struct._ZTS4Args.Args addrspace(4)* addrspace(4)* + store %struct._ZTS4Args.Args addrspace(4)* %args, %struct._ZTS4Args.Args addrspace(4)* addrspace(4)* %args.addr.ascast, align 8, !tbaa !5 + ret i32 0 +} + +; Function Attrs: convergent norecurse nounwind mustprogress +define dso_local spir_func i32 @_Z9somefunc1P6Objecti(%struct._ZTS6Object.Object addrspace(4)* %object, i32 %value) #0 { +entry: + %retval = alloca i32, align 4 + %retval.ascast = addrspacecast i32* %retval to i32 addrspace(4)* + %object.addr = alloca %struct._ZTS6Object.Object addrspace(4)*, align 8 + %object.addr.ascast = addrspacecast %struct._ZTS6Object.Object addrspace(4)** %object.addr to %struct._ZTS6Object.Object addrspace(4)* addrspace(4)* + %value.addr = alloca i32, align 4 + %value.addr.ascast = addrspacecast i32* %value.addr to i32 addrspace(4)* + store %struct._ZTS6Object.Object addrspace(4)* %object, %struct._ZTS6Object.Object addrspace(4)* addrspace(4)* %object.addr.ascast, align 8, !tbaa !5 + store i32 %value, i32 addrspace(4)* %value.addr.ascast, align 4, !tbaa !9 + ret i32 0 +} + +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" } + +!llvm.module.flags = !{!0, !1} +!opencl.spir.version = !{!2} +!spirv.Source = !{!3} +!llvm.ident = !{!4} + +!0 = !{i32 1, !"wchar_size", i32 4} +!1 = !{i32 7, !"frame-pointer", i32 2} +!2 = !{i32 1, i32 2} +!3 = !{i32 4, i32 100000} +!4 = !{!"clang version 13.0.0"} +!5 = !{!6, !6, i64 0} +!6 = !{!"any pointer", !7, i64 0} +!7 = !{!"omnipotent char", !8, i64 0} +!8 = !{!"Simple C++ TBAA"} +!9 = !{!10, !10, i64 0} +!10 = !{!"int", !7, i64 0} From 21a32a4e7941ac64081c80b5a5379316d91b0e0f Mon Sep 17 00:00:00 2001 From: Dmitry Sidorov Date: Tue, 1 Jun 2021 11:31:06 +0300 Subject: [PATCH 2/2] Simplify the test Signed-off-by: Dmitry Sidorov --- llvm-spirv/test/pointer_type_mapping.ll | 47 ++++--------------------- 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/llvm-spirv/test/pointer_type_mapping.ll b/llvm-spirv/test/pointer_type_mapping.ll index 7950cda7e529d..e76b360d1cd48 100644 --- a/llvm-spirv/test/pointer_type_mapping.ll +++ b/llvm-spirv/test/pointer_type_mapping.ll @@ -7,8 +7,8 @@ ; TODO add check count one and remove unused, when the type mapping bug is fixed ; CHECK: TypePointer [[#UNUSED:]] {{.*}} [[#NAME]] ; CHECK: TypePointer [[#PTRTY:]] {{.*}} [[#NAME]] -; CHECK-COUNT-2: TypePointer {{.*}} {{.*}} [[#PTRTY]] -; CHECK-NOT: TypePointer {{.*}} {{.*}} [[#UNUSED]] +; CHECK: FunctionParameter [[#PTRTY]] +; CHECK-NOT: FunctionParameter [[#UNUSED]] ; ModuleID = 'sycl_test.bc' source_filename = "sycl_test.cpp" @@ -19,45 +19,12 @@ target triple = "spir64-unknown-unknown-sycldevice" %struct._ZTS6Object.Object = type { i32 (%struct._ZTS6Object.Object addrspace(4)*, i32)* } ; Function Attrs: convergent norecurse nounwind mustprogress -define dso_local spir_func i32 @_Z9somefunc0P4Args(%struct._ZTS4Args.Args addrspace(4)* %args) #0 { +define dso_local spir_func i32 @_Z9somefunc0P4Args(%struct._ZTS4Args.Args addrspace(4)* %a, %struct._ZTS6Object.Object addrspace(4)* %b) { entry: - %retval = alloca i32, align 4 - %retval.ascast = addrspacecast i32* %retval to i32 addrspace(4)* - %args.addr = alloca %struct._ZTS4Args.Args addrspace(4)*, align 8 - %args.addr.ascast = addrspacecast %struct._ZTS4Args.Args addrspace(4)** %args.addr to %struct._ZTS4Args.Args addrspace(4)* addrspace(4)* - store %struct._ZTS4Args.Args addrspace(4)* %args, %struct._ZTS4Args.Args addrspace(4)* addrspace(4)* %args.addr.ascast, align 8, !tbaa !5 ret i32 0 } -; Function Attrs: convergent norecurse nounwind mustprogress -define dso_local spir_func i32 @_Z9somefunc1P6Objecti(%struct._ZTS6Object.Object addrspace(4)* %object, i32 %value) #0 { -entry: - %retval = alloca i32, align 4 - %retval.ascast = addrspacecast i32* %retval to i32 addrspace(4)* - %object.addr = alloca %struct._ZTS6Object.Object addrspace(4)*, align 8 - %object.addr.ascast = addrspacecast %struct._ZTS6Object.Object addrspace(4)** %object.addr to %struct._ZTS6Object.Object addrspace(4)* addrspace(4)* - %value.addr = alloca i32, align 4 - %value.addr.ascast = addrspacecast i32* %value.addr to i32 addrspace(4)* - store %struct._ZTS6Object.Object addrspace(4)* %object, %struct._ZTS6Object.Object addrspace(4)* addrspace(4)* %object.addr.ascast, align 8, !tbaa !5 - store i32 %value, i32 addrspace(4)* %value.addr.ascast, align 4, !tbaa !9 - ret i32 0 -} - -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" } - -!llvm.module.flags = !{!0, !1} -!opencl.spir.version = !{!2} -!spirv.Source = !{!3} -!llvm.ident = !{!4} - -!0 = !{i32 1, !"wchar_size", i32 4} -!1 = !{i32 7, !"frame-pointer", i32 2} -!2 = !{i32 1, i32 2} -!3 = !{i32 4, i32 100000} -!4 = !{!"clang version 13.0.0"} -!5 = !{!6, !6, i64 0} -!6 = !{!"any pointer", !7, i64 0} -!7 = !{!"omnipotent char", !8, i64 0} -!8 = !{!"Simple C++ TBAA"} -!9 = !{!10, !10, i64 0} -!10 = !{!"int", !7, i64 0} +!opencl.spir.version = !{!0} +!spirv.Source = !{!1} +!0 = !{i32 1, i32 2} +!1 = !{i32 4, i32 100000}