[SPIR-V] Add capability for non-uniform indexing for StructuredBuffer types.

[luciechoi]

• Capability StorageBufferArrayNonUniformIndexing is required if the non-uniform index accesses "arrays in the StorageBuffer storage class or BufferBlock-decorated arrays." (e.g. RWStructuredBuffer, StructuredBuffer)

• Also fix the wrong unit test name: StructuredBufferNonUniformIdx.ll -> RWBufferNonUniformIdx.ll

Resolves #162889
Addresses #161852

[luciechoi]

@s-perron

(is it possible if I can get a write access e.g. adding labels)

[llvmbot]

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

Author: Lucie Choi (luciechoi)

Changes

• Capability StorageBufferArrayNonUniformIndexing is required if the non-uniform index accesses "arrays in the StorageBuffer storage class or BufferBlock-decorated arrays."

• Also fix the wrong unit test name: StructuredBufferNonUniformIdx.ll -> RWBufferNonUniformIdx.ll

Resolves #162889

---

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

3 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp (+12-2)
• (renamed) llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWBufferNonUniformIdx.ll ()
• (modified) llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWStructuredBufferNonUniformIdx.ll (+1)

diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 5144fb14fa6a6..43efc6de5489b 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -1200,6 +1200,18 @@ void addOpAccessChainReqs(const MachineInstr &Instr,
     return;
   }
 
+  bool IsNonUniform =
+      hasNonUniformDecoration(Instr.getOperand(0).getReg(), MRI);
+  if (StorageClass == SPIRV::StorageClass::StorageClass::StorageBuffer) {
+    if (IsNonUniform)
+      Handler.addRequirements(
+          SPIRV::Capability::StorageBufferArrayNonUniformIndexingEXT);
+    else
+      Handler.addRequirements(
+          SPIRV::Capability::StorageBufferArrayDynamicIndexing);
+    return;
+  }
+
   Register PointeeTypeReg = ResTypeInst->getOperand(2).getReg();
   MachineInstr *PointeeType = MRI.getUniqueVRegDef(PointeeTypeReg);
   if (PointeeType->getOpcode() != SPIRV::OpTypeImage &&
@@ -1208,8 +1220,6 @@ void addOpAccessChainReqs(const MachineInstr &Instr,
     return;
   }
 
-  bool IsNonUniform =
-      hasNonUniformDecoration(Instr.getOperand(0).getReg(), MRI);
   if (isUniformTexelBuffer(PointeeType)) {
     if (IsNonUniform)
       Handler.addRequirements(
diff --git a/llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/StructuredBufferNonUniformIdx.ll b/llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWBufferNonUniformIdx.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/StructuredBufferNonUniformIdx.ll
rename to llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWBufferNonUniformIdx.ll
diff --git a/llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWStructuredBufferNonUniformIdx.ll b/llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWStructuredBufferNonUniformIdx.ll
index 2a12baf1e3ed4..a820e7a8ce06e 100644
--- a/llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWStructuredBufferNonUniformIdx.ll
+++ b/llvm/test/CodeGen/SPIRV/hlsl-resources/NonUniformIdx/RWStructuredBufferNonUniformIdx.ll
@@ -3,6 +3,7 @@
 
 ; CHECK-DAG: OpCapability Shader
 ; CHECK-DAG: OpCapability ShaderNonUniformEXT
+; CHECK-DAG: OpCapability StorageBufferArrayNonUniformIndexingEXT
 ; CHECK-DAG: OpDecorate {{%[0-9]+}} NonUniformEXT
 ; CHECK-DAG: OpDecorate {{%[0-9]+}} NonUniformEXT
 ; CHECK-DAG: OpDecorate {{%[0-9]+}} NonUniformEXT

[s-perron]

I found more bugs in the code in this function. Since we are trying to fix is up, can we get all of it fixed? We need to make sure that the index to the access chain is not a constant. For storage buffers, we need to pick out only the index into the array of storage buffers.

I think we can do that by making sure the input to the AC is a variable in the storage buffer storage class, and the type is an array.

I don't we have to worry about multi dimensional arrays. They are not allowed.

[s-perron]

I think this looks good. On my phone it's so hard to see everything clearly.

[luciechoi]

  uint32_t StorageClass = ResTypeInst->getOperand(1).getImm();
  if (StorageClass != SPIRV::StorageClass::StorageClass::UniformConstant &&
      StorageClass != SPIRV::StorageClass::StorageClass::Uniform &&
      StorageClass != SPIRV::StorageClass::StorageClass::StorageBuffer) {
    return;
  }

Verified this does guarantee access chain it's accessing is a resource, not a pointer to a resource