[spirv] Write values of resource types as a whole (#826)

Right now we just do a load and then a store over of whole struct,
completely ignoring the storage class (and thus layout). This is
surely invalid SPIR-V, but we need this right now to enable some
workloads. Hopefully SPIRV-Tools will be able to remove these
loads/stores.

Updating associated counters for certain types is a TODO.

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -391,10 +391,10 @@ uint32_t DeclResultIdMapper::createCTBuffer(const VarDecl *decl) {
   const auto usageKind = context->isCBuffer() ? ContextUsageKind::CBuffer
                                               : ContextUsageKind::TBuffer;
 
-  const std::string structName =
-      "type." +
-      std::string(context->isCBuffer() ? "ConstantBuffer." : "TextureBuffer.") +
-      recordType->getDecl()->getName().str();
+  const char *ctBufferName =
+      context->isCBuffer() ? "ConstantBuffer." : "TextureBuffer.";
+  const std::string structName = "type." + std::string(ctBufferName) +
+                                 recordType->getDecl()->getName().str();
   const uint32_t bufferVar = createVarOfExplicitLayoutStruct(
       recordType->getDecl(), usageKind, structName, decl->getName());
 

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -3251,14 +3251,24 @@ SpirvEvalInfo SPIRVEmitter::processAssignment(const Expr *lhs,
 void SPIRVEmitter::storeValue(const SpirvEvalInfo &lhsPtr,
                               const SpirvEvalInfo &rhsVal,
                               const QualType valType) {
-  // 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.
-  // TODO: is this optimization always correct?
-  if (lhsPtr.getLayoutRule() == rhsVal.getLayoutRule() ||
-      typeTranslator.isScalarType(valType) ||
+  if (typeTranslator.isScalarType(valType) ||
       typeTranslator.isVectorType(valType) ||
       typeTranslator.isMxNMatrix(valType)) {
     theBuilder.createStore(lhsPtr, rhsVal);
+  } 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.
+    // TODO: is this optimization always correct?
+    theBuilder.createStore(lhsPtr, rhsVal);
+  } else if (hlsl::IsHLSLResourceType(valType)) {
+    // Resource types are represented using RecordType in the AST.
+    // Handle them before the general RecordType.
+    //
+    // HLSL allows to put resource types in structs, or assign to variables
+    // of resource types. These can all result in illegal SPIR-V for Vulkan.
+    // We just translate here literally and let SPIRV-Tools opt to do the
+    // legalization work.
+    theBuilder.createStore(lhsPtr, rhsVal);
   } else if (const auto *recordType = valType->getAs<RecordType>()) {
     uint32_t index = 0;
     for (const auto *decl : recordType->getDecl()->decls()) {
@@ -3267,7 +3277,6 @@ void SPIRVEmitter::storeValue(const SpirvEvalInfo &lhsPtr,
         continue;
 
       const auto *field = cast<FieldDecl>(decl);
-      assert(field);
 
       const auto subRhsValType = typeTranslator.translateType(
           field->getType(), rhsVal.getLayoutRule());

tools/clang/test/CodeGenSPIRV/binary-op.assign.resource.hlsl

@@ -0,0 +1,106 @@
+// Run: %dxc -T vs_6_0 -E main
+
+// Note: The following is invalid SPIR-V code.
+//
+// * The assignment ignores storage class (and thus layout) difference.
+// * Associated counters for certain types are not assigned together.
+//
+// For the first, we will need SPIRV-Tools opt to legalize it.
+// For the second, it is a TODO.
+
+struct T {
+    float f;
+};
+
+struct ResourceBundle {
+    SamplerState               sampl;
+    Buffer<float3>             buf;
+    RWBuffer<float3>           rwbuf;
+    Texture2D<float>           tex2d;
+    RWTexture2D<float4>        rwtex2d;
+    // The frontend does not allow ConstantBuffers and TextureBuffers to be in a struct.
+    //ConstantBuffer<T>          cbuf;
+    //TextureBuffer<T>           tbuf;
+    StructuredBuffer<T>        sbuf;
+    RWStructuredBuffer<T>      rwsbuf;
+    AppendStructuredBuffer<T>  asbuf;
+    ConsumeStructuredBuffer<T> csbuf;
+    ByteAddressBuffer          babuf;
+    RWByteAddressBuffer        rwbabuf;
+};
+
+SamplerState               g_sampl;
+Buffer<float3>             g_buf;
+RWBuffer<float3>           g_rwbuf;
+Texture2D<float>           g_tex2d;
+RWTexture2D<float4>        g_rwtex2d;
+//ConstantBuffer<T>          g_cbuf;
+//TextureBuffer<T>           g_tbuf;
+StructuredBuffer<T>        g_sbuf;
+RWStructuredBuffer<T>      g_rwsbuf;
+AppendStructuredBuffer<T>  g_asbuf;
+ConsumeStructuredBuffer<T> g_csbuf;
+ByteAddressBuffer          g_babuf;
+RWByteAddressBuffer        g_rwbabuf;
+
+void main() {
+    ResourceBundle b;
+
+// CHECK:      [[val:%\d+]] = OpLoad %type_sampler %g_sampl
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_sampler %b %int_0
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.sampl   = g_sampl;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_buffer_image %g_buf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_buffer_image %b %int_1
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.buf     = g_buf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_buffer_image_0 %g_rwbuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_buffer_image_0 %b %int_2
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.rwbuf   = g_rwbuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_2d_image %g_tex2d
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_2d_image %b %int_3
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.tex2d   = g_tex2d;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_2d_image_0 %g_rwtex2d
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_2d_image_0 %b %int_4
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.rwtex2d = g_rwtex2d;
+
+    //b.cbuf    = g_cbuf;
+    //b.tbuf    = g_tbuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_StructuredBuffer_T %g_sbuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_StructuredBuffer_T_0 %b %int_5
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.sbuf    = g_sbuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_RWStructuredBuffer_T %g_rwsbuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_RWStructuredBuffer_T_0 %b %int_6
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.rwsbuf  = g_rwsbuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_RWStructuredBuffer_T %g_asbuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_RWStructuredBuffer_T_0 %b %int_7
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.asbuf   = g_asbuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_RWStructuredBuffer_T %g_csbuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_RWStructuredBuffer_T_0 %b %int_8
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.csbuf   = g_csbuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_ByteAddressBuffer %g_babuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_ByteAddressBuffer %b %int_9
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.babuf   = g_babuf;
+
+// CHECK-NEXT: [[val:%\d+]] = OpLoad %type_RWByteAddressBuffer %g_rwbabuf
+// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Function_type_RWByteAddressBuffer %b %int_10
+// CHECK-NEXT:                OpStore [[ptr]] [[val]]
+    b.rwbabuf = g_rwbabuf;
+}

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -138,6 +138,11 @@ TEST_F(FileTest, BinaryOpAssign) { runFileTest("binary-op.assign.hlsl"); }
 TEST_F(FileTest, BinaryOpAssignComposite) {
   runFileTest("binary-op.assign.composite.hlsl");
 }
+TEST_F(FileTest, BinaryOpAssignResource) {
+  runFileTest("binary-op.assign.resource.hlsl", Expect::Success,
+              // It generates invalid SPIR-V code right now.
+              /*noValidation=*/true);
+}
 
 // For comma binary operator
 TEST_F(FileTest, BinaryOpComma) { runFileTest("binary-op.comma.hlsl"); }

tools/clang/unittests/SPIRV/FileTestFixture.cpp

@@ -60,7 +60,8 @@ bool FileTest::parseInputFile() {
   return true;
 }
 
-void FileTest::runFileTest(llvm::StringRef filename, Expect expect) {
+void FileTest::runFileTest(llvm::StringRef filename, Expect expect,
+                           bool noValidation) {
   inputFilePath = utils::getAbsPathOfInputDataFile(filename);
 
   // Parse the input file.
@@ -123,7 +124,7 @@ void FileTest::runFileTest(llvm::StringRef filename, Expect expect) {
   ASSERT_EQ(result.status(), effcee::Result::Status::Ok);
 
   // Run SPIR-V validation for successful compilations
-  if (expect != Expect::Failure) {
+  if (!noValidation && expect != Expect::Failure) {
     EXPECT_TRUE(utils::validateSpirvBinary(generatedBinary));
   }
 }

tools/clang/unittests/SPIRV/FileTestFixture.h

@@ -26,7 +26,8 @@ public:
   };
 
   /// \brief Runs a File Test! (See class description for more info)
-  void runFileTest(llvm::StringRef path, Expect expect = Expect::Success);
+  void runFileTest(llvm::StringRef path, Expect expect = Expect::Success,
+                   bool noValidation = false);
 
 private:
   /// \brief Reads in the given input file.