[llvm] [SPIR-V] Fixup storage class for global private (PR #116636)

Nathan Gauër via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 18 07:58:22 PST 2024


================
@@ -3264,9 +3264,15 @@ bool SPIRVInstructionSelector::selectGlobalValue(
     PointerBaseType = GR.getOrCreateSPIRVType(
         GVType, MIRBuilder, SPIRV::AccessQualifier::ReadWrite, false);
   }
-  SPIRVType *ResType = GR.getOrCreateSPIRVPointerType(
-      PointerBaseType, I, TII,
-      addressSpaceToStorageClass(GV->getAddressSpace(), STI));
+
+  const unsigned AddrSpace = GV->getAddressSpace();
+  SPIRV::StorageClass::StorageClass StorageClass =
+      addressSpaceToStorageClass(AddrSpace, STI);
+  if (StorageClass == SPIRV::StorageClass::Function)
+    StorageClass = SPIRV::StorageClass::Private;
----------------
Keenuts wrote:

I agree this is a bit weird.
I think the function itself makes the assumption AS is enough to determine the storage class, while it's not correct. The function for example doesn't look at any visibility/storage attributes in the IR variable, just AS.

I see 2 ways to fix this in a more generic way:
 - remove this function, or pass the `llvm::Value` definition to correctly look at it.
 - use AS to be the generic vessel for storage class transmission, and patch the IR to use a specific AS.

https://github.com/llvm/llvm-project/pull/116636


More information about the llvm-commits mailing list