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

[Steven Perron via llvm-commits]

================
@@ -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;
----------------
s-perron wrote:

This is the tricky part. In some ways it makes sense. A global value then that is in `Function` scope is meaningless. The choice are that we assert or change the storage class. I don't know how that will affect the code gen for functions.

If we do change the storage class to private, is there any documentation that will need to be updated?

Could this adhoc change here cause problems if another locations calls `addressSpaceToStorageClass` to get the storage class? If they don't do the same switch could we get inconsistent results. For example, a function that returns the pointer to a variable? At the call site, they will only see an addressspace of 0. How will it know if it actually returns value is a global value (Private) or something else (Function). I'm just thinking off the top of my head. This needs to be carefully thought out.

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