[SPIR-V] Do not emit spv_ptrcast if GEP result is of expected type

[michalpaszkowski]

Prior to this change spv_ptrcast (and OpBitcast) was never emitted for GEP resulting pointers. While such SPIR-V was (mostly) accepted by the NEO GPU driver, the generated SPIR-V was incorrect.

The newly added test (pointers/getelementptr-bitcast-load.ll) verifies that a correct bitcast is added for more complex cases and passes spirv-val. The test is based on an OpenCL CTS test (basic/prefetch).

[llvmbot]

@llvm/pr-subscribers-backend-spir-v

Author: Michal Paszkowski (michalpaszkowski)

Changes

Prior to this change spv_ptrcast (and OpBitcast) was never emitted for GEP resulting pointers. While such SPIR-V was (mostly) accepted by the NEO GPU driver, the generated SPIR-V was incorrect.

The newly added test (pointers/getelementptr-bitcast-load.ll) verifies that a correct bitcast is added for more complex cases and passes spirv-val. The test is based on an OpenCL CTS test (basic/prefetch).

---

Full diff: https://github.com/llvm/llvm-project/pull/78122.diff

3 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp (+3-1)
• (modified) llvm/test/CodeGen/SPIRV/logical-struct-access.ll (+9-9)
• (added) llvm/test/CodeGen/SPIRV/pointers/getelementptr-bitcast-load.ll (+33)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index fb4e9932dd2dcf..90ec98bb361d3c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -314,7 +314,9 @@ void SPIRVEmitIntrinsics::insertPtrCastInstr(Instruction *I) {
   if (A && A->getAllocatedType() == ExpectedElementType)
     return;
 
-  if (dyn_cast<GetElementPtrInst>(Pointer))
+  // Do not emit spv_ptrcast if Pointer is a result of GEP of expected type.
+  GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Pointer);
+  if (GEPI && GEPI->getResultElementType() == ExpectedElementType)
     return;
 
   setInsertPointSkippingPhis(*IRB, I);
diff --git a/llvm/test/CodeGen/SPIRV/logical-struct-access.ll b/llvm/test/CodeGen/SPIRV/logical-struct-access.ll
index 617aa70ccdb7e6..a1ff1e0752e034 100644
--- a/llvm/test/CodeGen/SPIRV/logical-struct-access.ll
+++ b/llvm/test/CodeGen/SPIRV/logical-struct-access.ll
@@ -1,27 +1,27 @@
 ; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
 
-; CHECK: [[uint:%[0-9]+]] = OpTypeInt 32 0
+; CHECK-DAG: [[uint:%[0-9]+]] = OpTypeInt 32 0
 
 %A = type {
   i32,
   i32
 }
-; CHECK:    [[A:%[0-9]+]] = OpTypeStruct [[uint]] [[uint]]
+; CHECK-DAG:    [[A:%[0-9]+]] = OpTypeStruct [[uint]] [[uint]]
 
 %B = type {
   %A,
   i32,
   %A
 }
-; CHECK:    [[B:%[0-9]+]] = OpTypeStruct [[A]] [[uint]] [[A]]
+; CHECK-DAG:    [[B:%[0-9]+]] = OpTypeStruct [[A]] [[uint]] [[A]]
 
-; CHECK: [[uint_0:%[0-9]+]] = OpConstant [[uint]] 0
-; CHECK: [[uint_1:%[0-9]+]] = OpConstant [[uint]] 1
-; CHECK: [[uint_2:%[0-9]+]] = OpConstant [[uint]] 2
+; CHECK-DAG: [[uint_0:%[0-9]+]] = OpConstant [[uint]] 0
+; CHECK-DAG: [[uint_1:%[0-9]+]] = OpConstant [[uint]] 1
+; CHECK-DAG: [[uint_2:%[0-9]+]] = OpConstant [[uint]] 2
 
-; CHECK: [[ptr_uint:%[0-9]+]] = OpTypePointer Function [[uint]]
-; CHECK:    [[ptr_A:%[0-9]+]] = OpTypePointer Function [[A]]
-; CHECK:    [[ptr_B:%[0-9]+]] = OpTypePointer Function [[B]]
+; CHECK-DAG: [[ptr_uint:%[0-9]+]] = OpTypePointer Function [[uint]]
+; CHECK-DAG:    [[ptr_A:%[0-9]+]] = OpTypePointer Function [[A]]
+; CHECK-DAG:    [[ptr_B:%[0-9]+]] = OpTypePointer Function [[B]]
 
 define void @main() #1 {
 entry:
diff --git a/llvm/test/CodeGen/SPIRV/pointers/getelementptr-bitcast-load.ll b/llvm/test/CodeGen/SPIRV/pointers/getelementptr-bitcast-load.ll
new file mode 100644
index 00000000000000..132f10262432b2
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/pointers/getelementptr-bitcast-load.ll
@@ -0,0 +1,33 @@
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: %[[#INT8:]] = OpTypeInt 8 0
+; CHECK-DAG: %[[#VEC3:]] = OpTypeVector %[[#INT8]] 3
+; CHECK-DAG: %[[#VEC4:]] = OpTypeVector %[[#INT8]] 4
+; CHECK-DAG: %[[#PTR_VEC3:]] = OpTypePointer CrossWorkgroup %[[#VEC3]]
+; CHECK-DAG: %[[#PTR_VEC4:]] = OpTypePointer CrossWorkgroup %[[#VEC4]]
+
+; CHECK: %[[#AC1:]] = OpInBoundsPtrAccessChain %[[#PTR_VEC3]] %[[#]] %[[#]]
+; CHECK: %[[#BC1:]] = OpBitcast %[[#PTR_VEC4]] %[[#AC1]]
+; CHECK: %[[#LD1:]] = OpLoad %[[#VEC4]] %[[#BC1]] Aligned 4
+; CHECK: OpReturn
+
+define spir_kernel void @foo(ptr addrspace(1) %a, i64 %b) {
+  %index = getelementptr inbounds <3 x i8>, ptr addrspace(1) %a, i64 %b
+  %loadv = load <4 x i8>, ptr addrspace(1) %index, align 4
+  ret void
+}
+
+; CHECK: %[[#AC2:]] = OpInBoundsPtrAccessChain %[[#PTR_VEC3]] %[[#]] %[[#]]
+; CHECK: %[[#BC2:]] = OpBitcast %[[#PTR_VEC4]] %[[#AC2]]
+; CHECK: %[[#LD2:]] = OpLoad %[[#VEC4]] %[[#BC2]] Aligned 4
+; CHECK: OpReturn
+
+define spir_kernel void @bar(ptr addrspace(1) %a, i64 %b) {
+  %index = getelementptr inbounds <3 x i8>, ptr addrspace(1) %a, i64 %b
+; This redundant bitcast is left here itentionally to simulate the conversion
+; from older LLVM IR with typed pointers.
+  %cast = bitcast ptr addrspace(1) %index to ptr addrspace(1)
+  %loadv = load <4 x i8>, ptr addrspace(1) %cast, align 4
+  ret void
+}

[iliya-diyachkov]

It looks good to me.