[spirv] use rvalue for array index (#3616)

The newly introduced template support generates a template instance
of a struct method that has a slightly different AST from the one
with hard-coded type. The template instance misses LValueToRValue
cast for an array index. This PR lets SPIR-V backend uses rvalue for
an array index. If the array index expression is not rvalue, it reports
an error.

tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -6839,7 +6839,20 @@ const Expr *SpirvEmitter::collectArrayStructIndices(
     // The index into an array must be an integer number.
     const auto *idxExpr = indexing->getIdx();
     const auto idxExprType = idxExpr->getType();
-    SpirvInstruction *thisIndex = doExpr(idxExpr);
+
+    if (idxExpr->isLValue()) {
+      // If the given HLSL code is correct, this case will not happen, because
+      // the correct HLSL code will not use LValue for the index of an array.
+      emitError("Index of ArraySubscriptExpr must be rvalue",
+                idxExpr->getExprLoc());
+      return nullptr;
+    }
+    // Since `doExpr(idxExpr)` can generate LValue SPIR-V instruction for
+    // RValue `idxExpr` (see
+    // https://github.com/microsoft/DirectXShaderCompiler/issues/3620),
+    // we have to use `loadIfGLValue(idxExpr)` instead of `doExpr(idxExpr)`.
+    SpirvInstruction *thisIndex = loadIfGLValue(idxExpr);
+
     if (!idxExprType->isIntegerType() || idxExprType->isBooleanType()) {
       thisIndex = castToInt(thisIndex, idxExprType, astContext.UnsignedIntTy,
                             idxExpr->getExprLoc());

tools/clang/test/CodeGenSPIRV/use.rvalue.for.member-expr.of.array-subscript.hlsl

@@ -0,0 +1,46 @@
+// Run: %dxc -T cs_6_0 -E main -enable-templates
+
+// Tests that a rvalue is used for the index of ArraySubscriptExpr. The newly
+// introduced template support generates a template instance of
+// `BufferAccess::load(uint index)` that misses LValueToRValue cast for a
+// MemberExpr. We prevent an array subscript from using lvalue.
+
+[[vk::binding(0, 0)]] ByteAddressBuffer babuf[]: register(t0, space0);
+[[vk::binding(0, 0)]] RWByteAddressBuffer rwbuf[]: register(u0, space0);
+
+struct BufferAccess {
+  uint handle;
+
+  template<typename T>
+  T load(uint index) {
+    // CHECK: [[handle_ptr:%\d+]] = OpAccessChain %_ptr_Function_uint %param_this %int_0
+    // CHECK: [[handle:%\d+]] = OpLoad %uint [[handle_ptr]]
+    // CHECK: OpAccessChain %_ptr_Uniform_type_ByteAddressBuffer %babuf [[handle]]
+    return babuf[this.handle].Load<T>(index * sizeof(T));
+  }
+
+  template<typename T>
+  void store(uint index, T value) {
+    return rwbuf[this.handle].Store<T>(index * sizeof(T), value);
+  }
+};
+
+struct Data {
+  BufferAccess buf;
+  uint a0;
+  uint a1;
+  uint a2;
+};
+
+struct A {
+  uint x;
+};
+
+[[vk::push_constant]] ConstantBuffer<Data> cbuf;
+
+[numthreads(1, 1, 1)]
+void main(uint tid : SV_DispatchThreadId) {
+  A b = cbuf.buf.load<A>(0);
+  b.x = 12;
+  cbuf.buf.store<A>(0, b);
+}

tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -2454,6 +2454,12 @@ TEST_F(FileTest, MeshShadingNVAmplificationError4) {
   runFileTest("meshshading.nv.error3.amplification.hlsl", Expect::Failure);
 }
 
+TEST_F(FileTest, UseRValueForMemberExprOfArraySubscriptExpr) {
+  runFileTest("use.rvalue.for.member-expr.of.array-subscript.hlsl",
+              Expect::Success,
+              /* runValidation */ false);
+}
+
 // Test OpEntryPoint in the Vulkan1.2 target environment
 TEST_F(FileTest, Vk1p2EntryPoint) {
   runFileTest("vk.1p2.entry-point.hlsl");