[SPIR-V] Normalize inline SPIR-V for builtins

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8066,6 +8066,19 @@ def err_hlsl_vk_static_pointer_cast_type: Error<
   "vk::static_pointer_cast() content type must be base class of argument's content type">;
 def warn_spirv_node_shaders_experimental : Warning<
   "SPIR-V implementation of node shaders is experimental and subject to change">;
+
+def err_hlsl_spv_inline_builtin_redefinition
+    : Error<"%0 cannot be used on a variable already associated with the "
+            "built-in %1">;
+def err_hlsl_spv_inline_builtin_incompatible
+    : Error<"%0 incompatible with the BuiltIn %1">;
+def err_hlsl_spv_inline_location_redefinition
+    : Error<"invalid %0 : target already associated with another location %1">;
+def err_hlsl_spv_inline_invalid_decoration_single_operand_missing
+    : Error<"decoration %0 requires 1 operand">;
+def err_hlsl_spv_inline_storage_class_redefinition
+    : Error<"%0 cannot be used on a variable already associated to another "
+            "storage class %1">;
 // SPIRV Change Ends
 
 let CategoryName = "OpenMP Issue" in {

tools/clang/include/clang/Sema/SemaHLSL.h

@@ -139,6 +139,8 @@ GetBestViableFunction(clang::Sema &S, clang::SourceLocation Loc,
 bool ShouldSkipNRVO(clang::Sema &sema, clang::QualType returnType,
                     clang::VarDecl *VD, clang::FunctionDecl *FD);
 
+void NormalizeInlineSPIRVAttributes(clang::Sema &S, clang::Decl *D);
+
 /// <summary>Processes an attribute for a declaration.</summary>
 /// <param name="S">Sema with context.</param>
 /// <param name="D">Annotated declaration.</param>

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -1298,6 +1298,10 @@ SpirvVariable *DeclResultIdMapper::createExternVar(const VarDecl *var,
     return varInstr;
 
   const auto *bindingAttr = var->getAttr<VKBindingAttr>();
+
+  if (var->hasAttr<VKStorageClassExtAttr>() && !bindingAttr)
+    return varInstr;
+
   resourceVars.emplace_back(varInstr, var, loc, getResourceBinding(var),
                             bindingAttr, var->getAttr<VKCounterBindingAttr>());
 

tools/clang/lib/Sema/SemaDeclAttr.cpp

@@ -5326,6 +5326,11 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) {
     D->addAttr(*i);
   }
   // HLSL Change Ends
+
+  // SPIR-V Change Starts
+  if (LangOpts.HLSL)
+    hlsl::NormalizeInlineSPIRVAttributes(*this, D);
+  // SPIR-V Change Ends
 }
 
 /// Is the given declaration allowed to use a forbidden type?

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -48,10 +48,18 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+
+#ifdef ENABLE_SPIRV_CODEGEN
+// Enables functions like spv::BuiltInToString()
+#define SPV_ENABLE_UTILITY_CODE
+#include "spirv/unified1/spirv.hpp11"
+#endif
+
 #include <algorithm>
 #include <array>
 #include <bitset>
 #include <float.h>
+#include <optional>
 
 enum ArBasicKind {
   AR_BASIC_BOOL,
@@ -14593,6 +14601,162 @@ void ValidateDispatchGridValues(DiagnosticsEngine &Diags,
         << A.getName() << A.getRange();
 }
 
+void hlsl::NormalizeInlineSPIRVAttributes(Sema &S, Decl *D) {
+#ifdef ENABLE_SPIRV_CODEGEN
+  // Collecting the values that can be set across multiple attributes.
+  std::optional<std::pair<spv::StorageClass, SourceRange>> StorageClass;
+  std::optional<std::pair<unsigned, SourceRange>> Location;
+  std::optional<std::pair<spv::BuiltIn, SourceRange>> BuiltIn;
+
+  // Vector collecting all the other attributes.
+  clang::AttrVec NewAttrs;
+
+  for (auto *A : D->attrs()) {
+    if (auto *DA = dyn_cast<VKDecorateExtAttr>(A)) {
+      spv::Decoration Decoration = spv::Decoration(DA->getDecorate());
+      if (Decoration == spv::Decoration::Location) {
+        if (DA->literals_size() != 1) {
+          S.Diags.Report(
+              A->getLocation(),
+              diag::
+                  err_hlsl_spv_inline_invalid_decoration_single_operand_missing)
+              << "Location";
+          return;
+        }
+        unsigned Index = *DA->literals_begin();
+        if (Location.has_value() && Location->first != Index) {
+          S.Diags.Report(A->getLocation(),
+                         diag::err_hlsl_spv_inline_location_redefinition)
+              << DA << Location->first;
+          return;
+        }
+        Location = {Index, DA->getLocation()};
+      } else if (Decoration == spv::Decoration::BuiltIn) {
+        if (DA->literals_size() != 1) {
+          S.Diags.Report(
+              A->getLocation(),
+              diag::
+                  err_hlsl_spv_inline_invalid_decoration_single_operand_missing)
+              << "BuiltIn";
+          return;
+        }
+        spv::BuiltIn ID = spv::BuiltIn(*DA->literals_begin());
+        if (BuiltIn.has_value() && ID != BuiltIn->first) {
+          S.Diags.Report(A->getLocation(),
+                         diag::err_hlsl_spv_inline_builtin_redefinition)
+              << DA << spv::BuiltInToString(BuiltIn->first);
+          return;
+        }
+        BuiltIn = {ID, DA->getLocation()};
+      } else {
+        NewAttrs.push_back(A);
+      }
+    } else if (auto *SCA = dyn_cast<VKStorageClassExtAttr>(A)) {
+      spv::StorageClass SC = spv::StorageClass(SCA->getStclass());
+      if (StorageClass.has_value() && StorageClass->first != SC) {
+        S.Diags.Report(A->getLocation(),
+                       diag::err_hlsl_spv_inline_storage_class_redefinition)
+            << SCA << spv::StorageClassToString(StorageClass->first);
+        return;
+      }
+      StorageClass = {SC, SCA->getLocation()};
+    } else if (auto *LA = dyn_cast<VKLocationAttr>(A)) {
+      if (Location.has_value() &&
+          Location->first != static_cast<unsigned>(LA->getNumber())) {
+        S.Diags.Report(A->getLocation(),
+                       diag::err_hlsl_spv_inline_location_redefinition)
+            << LA << Location->first;
+        return;
+      }
+      Location = {LA->getNumber(), LA->getLocation()};
+    } else if (auto *BA = dyn_cast<VKExtBuiltinOutputAttr>(A)) {
+      if (StorageClass.has_value() &&
+          StorageClass->first != spv::StorageClass::Output) {
+        S.Diags.Report(A->getLocation(),
+                       diag::err_hlsl_spv_inline_storage_class_redefinition)
+            << BA << spv::StorageClassToString(StorageClass->first);
+        return;
+      }
+      StorageClass = {spv::StorageClass::Output, BA->getLocation()};
+
+      spv::BuiltIn ID = spv::BuiltIn(BA->getBuiltInID());
+      if (BuiltIn.has_value() && ID != BuiltIn->first) {
+        S.Diags.Report(A->getLocation(),
+                       diag::err_hlsl_spv_inline_builtin_redefinition)
+            << BA << spv::BuiltInToString(BuiltIn->first);
+        return;
+      }
+      BuiltIn = {ID, BA->getLocation()};
+
+    } else if (auto *BA = dyn_cast<VKExtBuiltinInputAttr>(A)) {
+      if (StorageClass.has_value() &&
+          StorageClass->first != spv::StorageClass::Input) {
+        S.Diags.Report(A->getLocation(),
+                       diag::err_hlsl_spv_inline_storage_class_redefinition)
+            << BA << spv::StorageClassToString(StorageClass->first);
+        return;
+      }
+      StorageClass = {spv::StorageClass::Input, BA->getLocation()};
+
+      spv::BuiltIn ID = spv::BuiltIn(BA->getBuiltInID());
+      if (BuiltIn.has_value() && ID != BuiltIn->first) {
+        S.Diags.Report(A->getLocation(),
+                       diag::err_hlsl_spv_inline_builtin_redefinition)
+            << BA << spv::BuiltInToString(BuiltIn->first);
+        return;
+      }
+      BuiltIn = {ID, BA->getLocation()};
+    } else {
+      NewAttrs.push_back(A);
+    }
+  }
+
+  if (BuiltIn.has_value()) {
+    // Location should not be set if a BuiltIn is requested.
+    if (Location.has_value()) {
+      S.Diags.Report(D->getLocation(),
+                     diag::err_hlsl_spv_inline_builtin_incompatible)
+          << "Location" << spv::BuiltInToString(BuiltIn->first);
+      return;
+    }
+
+    // BuiltIn requires either Input or Output storage class.
+    if (StorageClass.has_value() &&
+        StorageClass->first == spv::StorageClass::Input)
+      NewAttrs.push_back(new (S.Context) VKExtBuiltinInputAttr(
+          BuiltIn->second, S.Context, static_cast<unsigned>(BuiltIn->first),
+          0));
+    else if (StorageClass.has_value() &&
+             StorageClass->first == spv::StorageClass::Output)
+      NewAttrs.push_back(new (S.Context) VKExtBuiltinOutputAttr(
+          BuiltIn->second, S.Context, static_cast<unsigned>(BuiltIn->first),
+          0));
+    else if (isa<ParmVarDecl>(D) || isa<FunctionDecl>(D)) {
+      unsigned BuiltInID = static_cast<unsigned>(BuiltIn->first);
+      NewAttrs.push_back(new (S.Context) VKDecorateExtAttr(
+          BuiltIn->second, S.Context, /* BuiltIn */ 11, &BuiltInID, 1, 0));
+    } else
+      S.Diags.Report(D->getLocation(),
+                     diag::err_hlsl_spv_inline_builtin_incompatible)
+          << std::string("StorageClass ") +
+                 spv::StorageClassToString(StorageClass->first)
+          << spv::BuiltInToString(BuiltIn->first);
+  } else {
+    if (Location.has_value())
+      NewAttrs.push_back(new (S.Context) VKLocationAttr(
+          Location->second, S.Context, static_cast<unsigned>(Location->first),
+          0));
+    if (StorageClass.has_value())
+      NewAttrs.push_back(new (S.Context) VKStorageClassExtAttr(
+          StorageClass->second, S.Context,
+          static_cast<unsigned>(StorageClass->first), 0));
+  }
+
+  D->dropAttrs();
+  D->setAttrs(NewAttrs);
+#endif // ENABLE_SPIRV_CODEGEN
+}
+
 void hlsl::HandleDeclAttributeForHLSL(Sema &S, Decl *D, const AttributeList &A,
                                       bool &Handled) {
   DXASSERT_NOMSG(D != nullptr);

tools/clang/test/CodeGenSPIRV/inline-spirv/spv.raytracing.hlsl

@@ -2,6 +2,8 @@
 
 // CHECK-DAG: OpCapability RuntimeDescriptorArray
 // CHECK-DAG: OpCapability RayQueryKHR
+// CHECK-DAG: OpDecorate %MyScene DescriptorSet 3
+// CHECK-DAG: OpDecorate %MyScene Binding 2
 
 using A = vk::SpirvOpaqueType</* OpTypeAccelerationStructureKHR */ 5341>;
 // CHECK: %[[name:[^ ]+]] = OpTypeAccelerationStructureKHR
@@ -12,6 +14,8 @@ using RA [[vk::ext_capability(/* RuntimeDescriptorArray */ 5302)]] = vk::SpirvOp
 // CHECK: %[[ptr:[^ ]+]] = OpTypePointer UniformConstant %[[rarr]]
 // CHECK: %MyScene = OpVariable %[[ptr]] UniformConstant
 [[vk::ext_storage_class(0)]]
+[[vk::ext_decorate(/* Binding */ 33, 2)]]
+[[vk::ext_decorate(/* DescriptorSet */ 34, 3)]]
 RA MyScene;
 
 [numthreads(1, 1, 1)]

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.bad.storage.hlsl

@@ -0,0 +1,9 @@
+// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv -verify
+
+[[vk::ext_decorate(/* BuiltIn */ 11, /* WorkgroupId */ 26)]]
+[[vk::ext_storage_class(/* UniformConstant */ 0)]]
+// expected-error@+1{{StorageClass UniformConstant incompatible with the BuiltIn WorkgroupId}}
+static uint3 input;
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.both.error.hlsl

@@ -0,0 +1,9 @@
+// RUN: %dxc -T ps_6_0 -E main -fcgl %s -spirv -verify
+
+// expected-error@+1{{'ext_builtin_input' cannot be used on a variable already associated to another storage class Output}}
+[[vk::ext_builtin_input(/* NumWorkgroups */ 24)]]
+[[vk::ext_builtin_output(/* NumWorkgroups */ 24)]]
+static uint3 invalid;
+
+void main() {
+}

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.location.hlsl

@@ -0,0 +1,10 @@
+// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv -verify
+
+[[vk::ext_decorate(/* BuiltIn */ 11, /* WorkgroupId */ 26)]]
+[[vk::ext_builtin_input(/* WorkgroupId */ 26)]]
+[[vk::ext_decorate(/* Location */ 30, 0)]]
+// expected-error@+1{{Location incompatible with the BuiltIn WorkgroupId}}
+static uint3 input;
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.missing.hlsl

@@ -0,0 +1,8 @@
+// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv -verify
+
+// expected-error@+1{{decoration BuiltIn requires 1 operand}}
+[[vk::ext_decorate(/* BuiltIn */ 11)]]
+static uint3 input;
+
+[numthreads(1, 1, 1)]
+void main() {}

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.error.hlsl

@@ -0,0 +1,10 @@
+// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv -verify
+
+// expected-error@+1{{'ext_decorate' cannot be used on a variable already associated with the built-in LocalInvocationId}}
+[[vk::ext_decorate(/* BuiltIn */ 11, /* WorkgroupId */ 26)]]
+[[vk::ext_decorate(/* BuiltIn */ 11, /* LocalInvocationId */ 27)]]
+[[vk::ext_storage_class(/* Input */ 1)]]
+static uint3 invalid;
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.input.hlsl

@@ -0,0 +1,9 @@
+// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv -verify
+
+// expected-error@+1{{'ext_builtin_input' cannot be used on a variable already associated with the built-in WorkgroupId}}
+[[vk::ext_builtin_input(/* NumWorkgroups */ 24)]]
+[[vk::ext_builtin_input(/* WorkgroupId */ 26)]]
+static uint3 invalid;
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.input.manual.hlsl

@@ -0,0 +1,11 @@
+// RUN: %dxc -T cs_6_9 -E main -ast-dump -spirv %s | FileCheck %s
+
+[[vk::ext_decorate(/* BuiltIn */ 11, /* WorkgroupId */ 26)]]
+[[vk::ext_builtin_input(/* WorkgroupId */ 26)]]
+static uint3 input;
+
+// CHECK: VarDecl 0x{{.+}} <{{.+}}> col:14 input 'uint3':'vector<unsigned int, 3>' static
+// CHECK-NEXT: VKExtBuiltinInputAttr 0x{{.+}} <line:3:3> 26
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.input.storage.conflict.hlsl

@@ -0,0 +1,9 @@
+// RUN: %dxc -T cs_6_9 -E main -spirv %s -verify
+
+// expected-error@+1{{'ext_storage_class' cannot be used on a variable already associated to another storage class Input}}
+[[vk::ext_storage_class(/* Output */ 3)]]
+[[vk::ext_builtin_input(/* WorkgroupId */ 26)]]
+static uint3 input;
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.input.storage.hlsl

@@ -0,0 +1,10 @@
+// RUN: %dxc -T cs_6_9 -E main -ast-dump -spirv %s | FileCheck %s
+
+[[vk::ext_storage_class(/* Input */ 1)]]
+[[vk::ext_builtin_input(/* WorkgroupId */ 26)]]
+static uint3 input;
+// CHECK: VarDecl 0x{{.+}} <{{.+}}> col:14 input 'uint3':'vector<unsigned int, 3>' static
+// CHECK-NEXT: VKExtBuiltinInputAttr 0x{{.+}} <line:4:3> 26
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.input.storage2.hlsl

@@ -0,0 +1,10 @@
+// RUN: %dxc -T cs_6_9 -E main -ast-dump -spirv %s | FileCheck %s
+
+[[vk::ext_builtin_input(/* WorkgroupId */ 26)]]
+[[vk::ext_storage_class(/* Input */ 1)]]
+static uint3 input;
+// CHECK: VarDecl 0x{{.+}} <{{.+}}> col:14 input 'uint3':'vector<unsigned int, 3>' static
+// CHECK-NEXT: VKExtBuiltinInputAttr 0x{{.+}} <line:3:3> 26
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.output.hlsl

@@ -0,0 +1,9 @@
+// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv -verify
+
+// expected-error@+1{{'ext_builtin_output' cannot be used on a variable already associated with the built-in WorkgroupId}}
+[[vk::ext_builtin_output(/* NumWorkgroups */ 24)]]
+[[vk::ext_builtin_output(/* WorkgroupId */ 26)]]
+static uint3 invalid;
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.output.manual.hlsl

@@ -0,0 +1,11 @@
+// RUN: %dxc -T cs_6_9 -E main -ast-dump -spirv %s | FileCheck %s
+
+[[vk::ext_decorate(/* BuiltIn */ 11, /* WorkgroupId */ 26)]]
+[[vk::ext_builtin_output(/* WorkgroupId */ 26)]]
+static uint3 input;
+
+// CHECK: VarDecl 0x{{.+}} <{{.+}}> col:14 input 'uint3':'vector<unsigned int, 3>' static
+// CHECK-NEXT: VKExtBuiltinOutputAttr 0x{{.+}} <line:3:3> 26
+
+[numthreads(1, 1, 1)]
+void main() { }

tools/clang/test/SemaHLSL/inline-spirv/spv.inline.builtin.multiple.output.storage.conflict.hlsl

@@ -0,0 +1,10 @@
+// RUN: %dxc -T vs_6_9 -E main -spirv %s -verify
+
+// expected-error@+1{{'ext_storage_class' cannot be used on a variable already associated to another storage class Output}}
+[[vk::ext_storage_class(/* Input */ 1)]]
+[[vk::ext_builtin_output(/* Position */ 0)]]
+static float4 output;
+
+void main() {
+}
+