[SPIR-V] Stop generating StorageImageReadWithoutFormat and StorageImageWriteWithoutFormat for the Unknown image format in the OpenCL environment

[VyacheslavLevytskyy]

This PR resolves the issue of the SPIR-V specification, requiring Shader-coupled capabilities to read/write images in the OpenCL SPIR-V environment, from the perspective of the LLVM SPIR-V backend. See KhronosGroup/SPIRV-Headers#487 for details and discussion.

Current implementation correctly reproduces requirements of the SPIR-V specification, however, since the requirements are problematic, out current implementation blocks generation of valid SPIR-V code for compute environments. This PR is to implement a solution discussed at the SPIR-V WG to allow proceeding with generation of valid SPIR-V code for the OpenCL environment and do not impact Vulkan environment at the same time.

[llvmbot]

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

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

This PR resolves the issue of the SPIR-V specification, requiring Shader-coupled capabilities to read/write images in the OpenCL SPIR-V environment, from the perspective of the LLVM SPIR-V backend. See KhronosGroup/SPIRV-Headers#487 for details and discussion.

Current implementation correctly reproduces requirements of the SPIR-V specification, however, since the requirements are problematic, out current implementation blocks generation of valid SPIR-V code for compute environments. This PR is to implement a solution discussed at the SPIR-V WG to allow proceeding with generation of valid SPIR-V code for the OpenCL environment and do not impact Vulkan environment at the same time.

---

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

3 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp (+12-2)
• (modified) llvm/test/CodeGen/SPIRV/read_image.ll (+8-1)
• (modified) llvm/test/CodeGen/SPIRV/transcoding/OpImageReadMS.ll (+9-1)

diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index fea3965dd8f2a..b2c12411ab782 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -1723,7 +1723,12 @@ void addInstrRequirements(const MachineInstr &MI,
     Register ImageReg = MI.getOperand(2).getReg();
     SPIRVType *TypeDef = ST.getSPIRVGlobalRegistry()->getResultType(
         ImageReg, const_cast<MachineFunction *>(MI.getMF()));
-    if (isImageTypeWithUnknownFormat(TypeDef))
+    // OpImageRead and OpImageWrite can use Unknown Image Formats
+    // when the Kernel capability is declared. In the OpenCL environment we are
+    // not allowed to produce
+    // StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat, see
+    // https://github.com/KhronosGroup/SPIRV-Headers/issues/487
+    if (isImageTypeWithUnknownFormat(TypeDef) && !ST.isOpenCLEnv())
       Reqs.addCapability(SPIRV::Capability::StorageImageReadWithoutFormat);
     break;
   }
@@ -1731,7 +1736,12 @@ void addInstrRequirements(const MachineInstr &MI,
     Register ImageReg = MI.getOperand(0).getReg();
     SPIRVType *TypeDef = ST.getSPIRVGlobalRegistry()->getResultType(
         ImageReg, const_cast<MachineFunction *>(MI.getMF()));
-    if (isImageTypeWithUnknownFormat(TypeDef))
+    // OpImageRead and OpImageWrite can use Unknown Image Formats
+    // when the Kernel capability is declared. In the OpenCL environment we are
+    // not allowed to produce
+    // StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat, see
+    // https://github.com/KhronosGroup/SPIRV-Headers/issues/487
+    if (isImageTypeWithUnknownFormat(TypeDef) && !ST.isOpenCLEnv())
       Reqs.addCapability(SPIRV::Capability::StorageImageWriteWithoutFormat);
     break;
   }
diff --git a/llvm/test/CodeGen/SPIRV/read_image.ll b/llvm/test/CodeGen/SPIRV/read_image.ll
index 20063aa6b8b75..cdad16217aeb7 100644
--- a/llvm/test/CodeGen/SPIRV/read_image.ll
+++ b/llvm/test/CodeGen/SPIRV/read_image.ll
@@ -1,6 +1,13 @@
 ; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
 
-; CHECK-SPIRV: OpCapability StorageImageReadWithoutFormat
+; StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat implicitly
+; declare Shader, causing a SPIR-V module to be rejected by the OpenCL
+; run-time. See https://github.com/KhronosGroup/SPIRV-Headers/issues/487
+; De-facto, OpImageRead and OpImageWrite are allowed to use Unknown Image
+; Formats when the Kernel capability is declared. We reflect this behavior
+; in the test case, and leave the check under CHECK-SPIRV-NOT to track
+; the issue and follow-up its final resolution when ready.
+; CHECK-SPIRV-NOT: OpCapability StorageImageReadWithoutFormat
 
 ; CHECK-SPIRV: %[[#IntTy:]] = OpTypeInt
 ; CHECK-SPIRV: %[[#IVecTy:]] = OpTypeVector %[[#IntTy]]
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpImageReadMS.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpImageReadMS.ll
index 4de79a24e2136..db934c2f3636f 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/OpImageReadMS.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/OpImageReadMS.ll
@@ -1,6 +1,14 @@
 ; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
 
-; CHECK-SPIRV: OpCapability StorageImageReadWithoutFormat
+; StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat implicitly
+; declare Shader, causing a SPIR-V module to be rejected by the OpenCL
+; run-time. See https://github.com/KhronosGroup/SPIRV-Headers/issues/487
+; De-facto, OpImageRead and OpImageWrite are allowed to use Unknown Image
+; Formats when the Kernel capability is declared. We reflect this behavior
+; in the test case, and leave the check under CHECK-SPIRV-NOT to track
+; the issue and follow-up its final resolution when ready.
+; CHECK-SPIRV-NOT: OpCapability StorageImageReadWithoutFormat
+
 ; CHECK-SPIRV: %[[#]] = OpImageRead %[[#]] %[[#]] %[[#]] Sample %[[#]]
 
 define spir_kernel void @sample_test(target("spirv.Image", void, 1, 0, 0, 1, 0, 0, 0) %source, i32 %sampler, <4 x float> addrspace(1)* nocapture %results) {