[spirv] Add validation for vk::builtin usage (#854)

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -905,6 +905,9 @@ bool DeclResultIdMapper::createStageVars(
       return false;
     }
 
+    if (!validateVKBuiltins(decl, sigPoint))
+      return false;
+
     const auto *builtinAttr = decl->getAttr<VKBuiltInAttr>();
 
     // For VS/HS/DS, the PointSize builtin is handled in gl_PerVertex.
@@ -1657,6 +1660,60 @@ uint32_t DeclResultIdMapper::createSpirvStageVar(StageVar *stageVar,
   return 0;
 }
 
+bool DeclResultIdMapper::validateVKBuiltins(const DeclaratorDecl *decl,
+                                            const hlsl::SigPoint *sigPoint) {
+  bool success = true;
+
+  if (const auto *builtinAttr = decl->getAttr<VKBuiltInAttr>()) {
+    const auto declType = getTypeOrFnRetType(decl);
+    const auto loc = builtinAttr->getLocation();
+
+    if (decl->hasAttr<VKLocationAttr>()) {
+      emitError("cannot use vk::builtin and vk::location together", loc);
+      success = false;
+    }
+
+    const llvm::StringRef builtin = builtinAttr->getBuiltIn();
+
+    if (builtin == "HelperInvocation") {
+      if (!declType->isBooleanType()) {
+        emitError("HelperInvocation builtin must be of boolean type", loc);
+        success = false;
+      }
+
+      if (sigPoint->GetKind() != hlsl::SigPoint::Kind::PSIn) {
+        emitError(
+            "HelperInvocation builtin can only be used as pixel shader input",
+            loc);
+        success = false;
+      }
+    } else if (builtin == "PointSize") {
+      if (!declType->isFloatingType()) {
+        emitError("PointSize builtin must be of float type", loc);
+        success = false;
+      }
+
+      switch (sigPoint->GetKind()) {
+      case hlsl::SigPoint::Kind::VSOut:
+      case hlsl::SigPoint::Kind::HSCPIn:
+      case hlsl::SigPoint::Kind::HSCPOut:
+      case hlsl::SigPoint::Kind::DSCPIn:
+      case hlsl::SigPoint::Kind::DSOut:
+      case hlsl::SigPoint::Kind::GSVIn:
+      case hlsl::SigPoint::Kind::GSOut:
+      case hlsl::SigPoint::Kind::PSIn:
+        break;
+      default:
+        emitError("PointSize builtin cannot be used as %0", loc)
+            << sigPoint->GetName();
+        success = false;
+      }
+    }
+  }
+
+  return success;
+}
+
 spv::StorageClass
 DeclResultIdMapper::getStorageClassForSigPoint(const hlsl::SigPoint *sigPoint) {
   // This translation is done based on the HLSL reference (see docs/dxil.rst).

tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -406,6 +406,10 @@ private:
   uint32_t createSpirvStageVar(StageVar *, const DeclaratorDecl *decl,
                                const llvm::StringRef name, SourceLocation);
 
+  /// Returns true if all vk::builtin usages are valid.
+  bool validateVKBuiltins(const DeclaratorDecl *decl,
+                          const hlsl::SigPoint *sigPoint);
+
   /// Creates the associated counter variable for RW/Append/Consume
   /// structured buffer.
   uint32_t createCounterVar(const ValueDecl *decl);

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -341,7 +341,7 @@ void SPIRVEmitter::HandleTranslationUnit(ASTContext &context) {
                 << bufferDecl->isCBuffer() << init->getSourceRange();
       }
 
-      validateVKAttributes(decl);
+      validateVKAttributes(bufferDecl);
 
       (void)declIdMapper.createCTBuffer(bufferDecl);
     }
@@ -702,7 +702,7 @@ void SPIRVEmitter::doFunctionDecl(const FunctionDecl *decl) {
   theBuilder.endFunction();
 }
 
-void SPIRVEmitter::validateVKAttributes(const Decl *decl) {
+void SPIRVEmitter::validateVKAttributes(const NamedDecl *decl) {
   // The frontend will make sure that
   // * vk::push_constant applies to global variables of struct type
   // * vk::binding applies to global variables or cbuffers/tbuffers
@@ -713,16 +713,17 @@ void SPIRVEmitter::validateVKAttributes(const Decl *decl) {
   // vk::binding.
 
   if (const auto *pcAttr = decl->getAttr<VKPushConstantAttr>()) {
+    const auto loc = pcAttr->getLocation();
+
     if (seenPushConstantAt.isInvalid()) {
-      seenPushConstantAt = pcAttr->getLocation();
+      seenPushConstantAt = loc;
     } else {
       // TODO: Actually this is slightly incorrect. The Vulkan spec says:
       //   There must be no more than one push constant block statically used
       //   per shader entry point.
       // But we are checking whether there are more than one push constant
       // blocks defined. Tracking usage requires more work.
-      emitError("cannot have more than one push constant block",
-                pcAttr->getLocation());
+      emitError("cannot have more than one push constant block", loc);
       emitNote("push constant block previously defined here",
                seenPushConstantAt);
     }
@@ -730,7 +731,7 @@ void SPIRVEmitter::validateVKAttributes(const Decl *decl) {
     if (decl->hasAttr<VKBindingAttr>()) {
       emitError("'push_constant' attribute cannot be used together with "
                 "'binding' attribute",
-                pcAttr->getLocation());
+                loc);
     }
   }
 }
@@ -4058,7 +4059,6 @@ SPIRVEmitter::processMatrixBinaryOp(const Expr *lhs, const Expr *rhs,
       const auto valId =
           theBuilder.createBinaryOp(spvOp, vecType, lhsVec, rhsVec);
       return SpirvEvalInfo(valId).setRValue();
-
     };
     return processEachVectorInMatrix(lhs, lhsVal, actOnEachVec);
   }

tools/clang/lib/SPIRV/SPIRVEmitter.h

@@ -240,7 +240,7 @@ private:
 
 private:
   /// Validates that vk::* attributes are used correctly.
-  void validateVKAttributes(const Decl *decl);
+  void validateVKAttributes(const NamedDecl *decl);
 
 private:
   /// Processes the given expr, casts the result into the given bool (vector)

tools/clang/test/CodeGenSPIRV/spirv.builtin.helper-invocation.invalid.hlsl

@@ -0,0 +1,11 @@
+// Run: %dxc -T vs_6_0 -E main
+
+[[vk::location(5), vk::builtin("HelperInvocation")]]
+int main() : A
+{
+    return 1;
+}
+
+// CHECK: :3:20: error: cannot use vk::builtin and vk::location together
+// CHECK: :3:20: error: HelperInvocation builtin must be of boolean type
+// CHECK: :3:20: error: HelperInvocation builtin can only be used as pixel shader input

tools/clang/test/CodeGenSPIRV/spirv.builtin.point-size.invalid.hlsl

@@ -0,0 +1,9 @@
+// Run: %dxc -T vs_6_0 -E main
+
+float main([[vk::location(5), vk::builtin("PointSize")]] int input : A) : B {
+    return input;
+}
+
+// CHECK: :3:31: error: cannot use vk::builtin and vk::location together
+// CHECK: :3:31: error: PointSize builtin must be of float type
+// CHECK: :3:31: error: PointSize builtin cannot be used as VSIn

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -942,6 +942,12 @@ TEST_F(FileTest, SpirvEntryFunctionInOut) {
 TEST_F(FileTest, SpirvBuiltInHelperInvocation) {
   runFileTest("spirv.builtin.helper-invocation.hlsl");
 }
+TEST_F(FileTest, SpirvBuiltInHelperInvocationInvalidUsage) {
+  runFileTest("spirv.builtin.helper-invocation.invalid.hlsl", Expect::Failure);
+}
+TEST_F(FileTest, SpirvBuiltInPointSizeInvalidUsage) {
+  runFileTest("spirv.builtin.point-size.invalid.hlsl", Expect::Failure);
+}
 
 // For shader stage input/output interface
 // For semantic SV_Position, SV_ClipDistance, SV_CullDistance