[llvm] 9e02e8f - fix producing multiple identical opaque pointer types (#79060)

[via llvm-commits]

Author: Vyacheslav Levytskyy
Date: 2024-01-30T18:11:53+01:00
New Revision: 9e02e8f1a76693f001c7f52ffddd92c492a2254e

URL: https://github.com/llvm/llvm-project/commit/9e02e8f1a76693f001c7f52ffddd92c492a2254e
DIFF: https://github.com/llvm/llvm-project/commit/9e02e8f1a76693f001c7f52ffddd92c492a2254e.diff

LOG: fix producing multiple identical opaque pointer types (#79060)

This PR fixes https://github.com/llvm/llvm-project/issues/79057 and
improves code generation for opaque pointers by replacing the culprit
SPIRVGlobalRegistry::getOpTypePointer() call with a more appropriate
SPIRVGlobalRegistry::getOrCreateSPIRVPointerType() call. The latter
function works together with the `DuplicatesTracker`
(`SPIRVGeneralDuplicatesTracker DT;` from `class SPIRVGlobalRegistry`)
to trace existence of previous definitions of opaque pointers. This
allows to produce just one `OpTypePointer` command for all identical
opaque pointers definitions and to return the very same type record for
subsequent `SPIRVGlobalRegistry::createSPIRVType()` invocations.

This PR alone improves code generation by producing a single needed
definition per all opaque pointers to i8 of the same address space
instead of multiple identical definitions produced before the patch.
>From the root cause analysis of
https://github.com/llvm/llvm-project/issues/79057 we see also that this
PR resolves the problem of inconsistency between keeping multiple
instruction for identical opaque pointer types and just a single record
for all such instructions in the `DuplicatesTracker`, and so it also
resolves the issue with crashes on creation of a struct with opaque
pointer fields due to the fact that now such struct fields refer to the
same operand `<id>` having a required record in the data structure used
for dependencies analysis (see
https://github.com/llvm/llvm-project/issues/79057).

Added: 
    llvm/test/CodeGen/SPIRV/pointers/struct-opaque-pointers.ll

Modified: 
    llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
    llvm/test/CodeGen/SPIRV/opencl/device_execution/execute_block.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index 6c009b9e8ddef..f9581390c28b9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -716,13 +716,14 @@ SPIRVType *SPIRVGlobalRegistry::createSPIRVType(
         ForwardPointerTypes[PType] = getOpTypeForwardPointer(SC, MIRBuilder);
       return ForwardPointerTypes[PType];
     }
-    Register Reg(0);
     // If we have forward pointer associated with this type, use its register
     // operand to create OpTypePointer.
-    if (ForwardPointerTypes.contains(PType))
-      Reg = getSPIRVTypeID(ForwardPointerTypes[PType]);
+    if (ForwardPointerTypes.contains(PType)) {
+      Register Reg = getSPIRVTypeID(ForwardPointerTypes[PType]);
+      return getOpTypePointer(SC, SpvElementType, MIRBuilder, Reg);
+    }
 
-    return getOpTypePointer(SC, SpvElementType, MIRBuilder, Reg);
+    return getOrCreateSPIRVPointerType(SpvElementType, MIRBuilder, SC);
   }
   llvm_unreachable("Unable to convert LLVM type to SPIRVType");
 }

diff  --git a/llvm/test/CodeGen/SPIRV/opencl/device_execution/execute_block.ll b/llvm/test/CodeGen/SPIRV/opencl/device_execution/execute_block.ll
index 1df72dc734dbd..562f5c7b6826e 100644
--- a/llvm/test/CodeGen/SPIRV/opencl/device_execution/execute_block.ll
+++ b/llvm/test/CodeGen/SPIRV/opencl/device_execution/execute_block.ll
@@ -1,7 +1,5 @@
 ; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
 
-; TODO(#60133): Requires updates following opaque pointer migration.
-; XFAIL: *
 ; REQUIRES: asserts
 
 ; CHECK: %[[#bool:]] = OpTypeBool

diff  --git a/llvm/test/CodeGen/SPIRV/pointers/struct-opaque-pointers.ll b/llvm/test/CodeGen/SPIRV/pointers/struct-opaque-pointers.ll
new file mode 100644
index 0000000000000..86f5f5bf24f5b
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/pointers/struct-opaque-pointers.ll
@@ -0,0 +1,16 @@
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK: %[[TyInt8:.*]] = OpTypeInt 8 0
+; CHECK: %[[TyInt8Ptr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyInt8]]
+; CHECK: %[[TyStruct:.*]] = OpTypeStruct %[[TyInt8Ptr]] %[[TyInt8Ptr]]
+; CHECK: %[[ConstStruct:.*]] = OpConstantComposite %[[TyStruct]] %[[ConstField:.*]] %[[ConstField]]
+; CHECK: %[[TyStructPtr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyStruct]]
+; CHECK: OpVariable %[[TyStructPtr]] {{[a-zA-Z]+}} %[[ConstStruct]]
+
+ at a = addrspace(1) constant i32 123
+ at struct = addrspace(1) global {ptr addrspace(1), ptr addrspace(1)} { ptr addrspace(1) @a, ptr addrspace(1) @a }
+
+define spir_kernel void @foo() {
+  ret void
+}