[spirv] Do per-element load for opaque arrays (#1189)

Copy opaque arrays (arrays of SPIR-V images and samplers) by
loading each element separately via access chains, then creating
a composite and write out once.

This is to aid SPIR-V transformations to avoid loading an array
of opaque objects as a whole.

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -796,6 +796,13 @@ SpirvEvalInfo SPIRVEmitter::loadIfGLValue(const Expr *expr,
   if (info.isRValue())
     return info;
 
+  // Check whether we are trying to load an array of opaque objects as a whole.
+  // If true, we are likely to copy it as a whole. To assist per-element
+  // copying, avoid the load here and return the pointer directly.
+  // TODO: consider moving this hack into SPIRV-Tools as a transformation.
+  if (TypeTranslator::isOpaqueArrayType(expr->getType()))
+    return info;
+
   // Check whether we are trying to load an externally visible structured/byte
   // buffer as a whole. If true, it means we are creating alias for it. Avoid
   // the load and write the pointer directly to the alias variable then.
@@ -4723,6 +4730,40 @@ void SPIRVEmitter::storeValue(const SpirvEvalInfo &lhsPtr,
     // assignments/returns from ConstantBuffer<T>/TextureBuffer<T> to function
     // parameters/returns/variables of type T. And ConstantBuffer<T> is not
     // represented differently as struct T.
+  } else if (TypeTranslator::isOpaqueArrayType(lhsValType)) {
+    // For opaque array types, we cannot perform OpLoad on the whole array and
+    // then write out as a whole; instead, we need to OpLoad each element
+    // using access chains. This is to influence later SPIR-V transformations
+    // to use access chains to access each opaque object; if we do array
+    // wholesale handling here, they will be in the final transformed code.
+    // Drivers don't like that.
+    // TODO: consider moving this hack into SPIRV-Tools as a transformation.
+    assert(lhsValType->isConstantArrayType());
+    assert(!rhsVal.isRValue());
+
+    const auto *arrayType = astContext.getAsConstantArrayType(lhsValType);
+    const auto elemType = arrayType->getElementType();
+    const auto arraySize =
+        static_cast<uint32_t>(arrayType->getSize().getZExtValue());
+
+    // Do separate load of each element via access chain
+    llvm::SmallVector<uint32_t, 8> elements;
+    for (uint32_t i = 0; i < arraySize; ++i) {
+      const auto subRhsValType =
+          typeTranslator.translateType(elemType, rhsVal.getLayoutRule());
+      const auto subRhsPtrType =
+          theBuilder.getPointerType(subRhsValType, rhsVal.getStorageClass());
+      const auto subRhsPtr = theBuilder.createAccessChain(
+          subRhsPtrType, rhsVal, {theBuilder.getConstantInt32(i)});
+
+      elements.push_back(theBuilder.createLoad(subRhsValType, subRhsPtr));
+    }
+
+    // Create a new composite and write out once
+    const auto lhsValTypeId =
+        typeTranslator.translateType(lhsValType, lhsPtr.getLayoutRule());
+    theBuilder.createStore(
+        lhsPtr, theBuilder.createCompositeConstruct(lhsValTypeId, elements));
   } else if (lhsPtr.getLayoutRule() == rhsVal.getLayoutRule()) {
     // If lhs and rhs has the same memory layout, we should be safe to load
     // from rhs and directly store into lhs and avoid decomposing rhs.

tools/clang/lib/SPIRV/TypeTranslator.cpp

@@ -158,6 +158,12 @@ bool TypeTranslator::isOpaqueStructType(QualType type) {
   return false;
 }
 
+bool TypeTranslator::isOpaqueArrayType(QualType type) {
+  if (const auto* arrayType = type->getAsArrayTypeUnsafe())
+    return isOpaqueType(arrayType->getElementType());
+  return false;
+}
+
 void TypeTranslator::LiteralTypeHint::setHint(QualType ty) {
   // You can set hint only once for each object.
   assert(type == QualType());

tools/clang/lib/SPIRV/TypeTranslator.h

@@ -219,6 +219,10 @@ public:
   /// Note: legalization specific code
   static bool isOpaqueType(QualType type);
 
+  /// Returns true if the given type will be translated into a array of SPIR-V
+  /// images or samplers.
+  static bool isOpaqueArrayType(QualType type);
+
   /// Returns true if the given type is a struct type who has an opaque field
   /// (in a recursive away).
   ///

tools/clang/test/CodeGenSPIRV/binary-op.assign.opaque.array.hlsl

@@ -0,0 +1,65 @@
+// Run: %dxc -T ps_6_0 -E main
+
+Texture2D    gTextures[1];
+SamplerState gSamplers[2];
+
+// Copy to static variable
+// CHECK:      [[src:%\d+]] = OpAccessChain %_ptr_UniformConstant_type_2d_image %gTextures %int_0
+// CHECK-NEXT: [[elm:%\d+]] = OpLoad %type_2d_image [[src]]
+// CHECK-NEXT: [[val:%\d+]] = OpCompositeConstruct %_arr_type_2d_image_uint_1 [[elm]]
+// CHECK-NEXT:                OpStore %sTextures [[val]]
+static Texture2D sTextures[1] = gTextures;
+
+struct Samplers {
+    SamplerState samplers[2];
+};
+
+struct Resources {
+    Texture2D textures[1];
+    Samplers  samplers;
+};
+
+float4 doSample(Texture2D t, SamplerState s[2]);
+
+float4 main() : SV_Target {
+    Resources r;
+    // Copy to struct field
+// CHECK:      OpAccessChain %_ptr_UniformConstant_type_2d_image %gTextures %int_0
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpCompositeConstruct %_arr_type_2d_image_uint_1
+    r.textures          = gTextures;
+
+// CHECK:      OpAccessChain %_ptr_UniformConstant_type_sampler %gSamplers %int_0
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpAccessChain %_ptr_UniformConstant_type_sampler %gSamplers %int_1
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpCompositeConstruct %_arr_type_sampler_uint_2
+    r.samplers.samplers = gSamplers;
+
+    // Copy to local variable
+// CHECK:      [[r:%\d+]] = OpAccessChain %_ptr_Function__arr_type_2d_image_uint_1 %r %int_0
+// CHECK-NEXT: OpAccessChain %_ptr_Function_type_2d_image [[r]] %int_0
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpCompositeConstruct %_arr_type_2d_image_uint_1
+    Texture2D    textures[1] = r.textures;
+    SamplerState samplers[2];
+// CHECK:      [[r:%\d+]] = OpAccessChain %_ptr_Function__arr_type_sampler_uint_2 %r %int_1 %int_0
+// CHECK-NEXT: OpAccessChain %_ptr_Function_type_sampler [[r]] %int_0
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpAccessChain %_ptr_Function_type_sampler [[r]] %int_1
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpCompositeConstruct %_arr_type_sampler_uint_2
+    samplers = r.samplers.samplers;
+
+// Copy to function parameter
+// CHECK:      OpAccessChain %_ptr_Function_type_sampler %samplers %int_0
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpAccessChain %_ptr_Function_type_sampler %samplers %int_1
+// CHECK-NEXT: OpLoad
+// CHECK-NEXT: OpCompositeConstruct %_arr_type_sampler_uint_2
+    return doSample(textures[0], samplers);
+}
+
+float4 doSample(Texture2D t, SamplerState s[2]) {
+    return t.Sample(s[1], float2(0.1, 0.2));
+}

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -173,6 +173,11 @@ TEST_F(FileTest, BinaryOpAssignImage) {
 TEST_F(FileTest, BinaryOpAssignComposite) {
   runFileTest("binary-op.assign.composite.hlsl");
 }
+TEST_F(FileTest, BinaryOpAssignOpaqueArray) {
+  // Test that for copying opaque arrays, we load each element via access chain
+  // separately, create an composite, and then write out once
+  runFileTest("binary-op.assign.opaque.array.hlsl");
+}
 
 // For comma binary operator
 TEST_F(FileTest, BinaryOpComma) { runFileTest("binary-op.comma.hlsl"); }