[spirv] Emit better errors on unsupported resource type parameter (#1381)

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -3156,7 +3156,7 @@ SpirvEvalInfo SPIRVEmitter::processBufferTextureLoad(
   } else if (elemType->isUnsignedIntegerType()) {
     elemTypeId = theBuilder.getUint32Type();
   } else {
-    emitError("buffer/texture type unimplemented", object->getExprLoc());
+    emitError("loading %0 value unsupported", object->getExprLoc()) << type;
     return 0;
   }
 

tools/clang/lib/SPIRV/TypeTranslator.cpp

@@ -1045,7 +1045,8 @@ bool TypeTranslator::isOrContainsNonFpColMajorMatrix(QualType type,
                                                      const Decl *decl) const {
   const auto isColMajorDecl = [this](const Decl *decl) {
     return decl->hasAttr<HLSLColumnMajorAttr>() ||
-           (!decl->hasAttr<HLSLRowMajorAttr>() && !spirvOptions.defaultRowMajor);
+           (!decl->hasAttr<HLSLRowMajorAttr>() &&
+            !spirvOptions.defaultRowMajor);
   };
 
   QualType elemType = {};
@@ -1191,12 +1192,20 @@ QualType TypeTranslator::getElementType(QualType type) {
   QualType elemType = {};
   if (isScalarType(type, &elemType) || isVectorType(type, &elemType) ||
       isMxNMatrix(type, &elemType)) {
-  } else if (const auto *arrType = astContext.getAsConstantArrayType(type)) {
-    elemType = arrType->getElementType();
-  } else {
-    assert(false && "unhandled type");
+    return elemType;
   }
-  return elemType;
+
+  if (const auto *arrType = astContext.getAsConstantArrayType(type)) {
+    return arrType->getElementType();
+  }
+
+  emitError("unsupported resource type parameter %0") << type;
+  // Note: We are returning the original type instead of a null QualType here
+  // to keep the translation going and avoid hitting asserts trying to query
+  // info from null QualType in other places of the compiler. Although we are
+  // likely generating invalid code here, it should be fine since the error
+  // reported will prevent the CodeGen from actually outputing.
+  return type;
 }
 
 uint32_t TypeTranslator::getComponentVectorType(QualType matrixType) {
@@ -1533,6 +1542,16 @@ uint32_t TypeTranslator::translateResourceType(QualType type, LayoutRule rule) {
   if (name == "Buffer" || name == "RWBuffer") {
     theBuilder.requireCapability(spv::Capability::SampledBuffer);
     const auto sampledType = hlsl::GetHLSLResourceResultType(type);
+    if (sampledType->isStructureType() && name.startswith("RW")) {
+      // Note: actually fxc supports RWBuffer over struct types. However, the
+      // struct member must fit into a 4-component vector and writing to a
+      // RWBuffer element must write all components. This is a feature that are
+      // rarely used by developers. We just emit an error saying not supported
+      // for now.
+      emitError("cannot instantiate RWBuffer with struct type %0")
+          << sampledType;
+      return 0;
+    }
     const auto format = translateSampledTypeToImageFormat(sampledType);
     return theBuilder.getImageType(
         translateType(getElementType(sampledType)), spv::Dim::Buffer,
@@ -1599,7 +1618,9 @@ TypeTranslator::translateSampledTypeToImageFormat(QualType sampledType) {
       }
     }
   }
-  emitError("resource type %0 unimplemented") << sampledType.getAsString();
+  emitError(
+      "cannot translate resource type parameter %0 to proper image format")
+      << sampledType;
   return spv::ImageFormat::Unknown;
 }
 

tools/clang/test/CodeGenSPIRV/type.buffer.struct.error.hlsl

@@ -0,0 +1,17 @@
+// Run: %dxc -T vs_6_0 -E main
+
+struct S {
+  float a;
+  float b;
+};
+
+// CHECK: error: cannot translate resource type parameter 'S' to proper image format
+// CHECK: error: unsupported resource type parameter 'S'
+  Buffer<S> MyBuffer;
+
+// CHECK: error: cannot instantiate RWBuffer with struct type 'S'
+RWBuffer<S> MyRWBuffer;
+
+float4 main() : A {
+  return 1.0;
+}

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -63,6 +63,9 @@ TEST_F(FileTest, SamplerTypes) { runFileTest("type.sampler.hlsl"); }
 TEST_F(FileTest, TextureTypes) { runFileTest("type.texture.hlsl"); }
 TEST_F(FileTest, RWTextureTypes) { runFileTest("type.rwtexture.hlsl"); }
 TEST_F(FileTest, BufferType) { runFileTest("type.buffer.hlsl"); }
+TEST_F(FileTest, BufferTypeStructError) {
+  runFileTest("type.buffer.struct.error.hlsl", Expect::Failure);
+}
 TEST_F(FileTest, CBufferType) { runFileTest("type.cbuffer.hlsl"); }
 TEST_F(FileTest, ConstantBufferType) {
   runFileTest("type.constant-buffer.hlsl");