[spirv] Prevent overlapping explicit locations (microsoft#5557)

When explicit location number assignments are used, ensure that if a
variable consumes more than one Location (for example, in the case of
double4) that all Locations are marked as used so that error messages
are correctly emitted.

Small improvements to error messaging and fixed up existing affected
tests while migrating them to LIT at the same time.

Fixes microsoft#5540

Author: sudonatalie

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -1932,6 +1932,7 @@ bool DeclResultIdMapper::finalizeStageIOLocations(bool forInput) {
 
       const auto *attr = var.getLocationAttr();
       const auto loc = attr->getNumber();
+      const auto locCount = var.getLocationCount();
       const auto attrLoc = attr->getLocation(); // Attr source code location
       const auto idx = var.getIndexAttr() ? var.getIndexAttr()->getNumber() : 0;
 
@@ -1943,13 +1944,17 @@ bool DeclResultIdMapper::finalizeStageIOLocations(bool forInput) {
       }
 
       // Make sure the same location is not assigned more than once
-      if (locSet.isLocUsed(loc, idx)) {
-        emitError("stage %select{output|input}0 location #%1 already assigned",
-                  attrLoc)
-            << forInput << loc;
-        noError = false;
+      for (uint32_t l = loc; l < loc + locCount; ++l) {
+        if (locSet.isLocUsed(l, idx)) {
+          emitError("stage %select{output|input}0 location #%1 already "
+                    "consumed by semantic '%2'",
+                    attrLoc)
+              << forInput << l << stageVars[idx].getSemanticStr();
+          noError = false;
+        }
+
+        locSet.useLoc(l, idx);
       }
-      locSet.useLoc(loc, idx);
 
       spvBuilder.decorateLocation(var.getSpirvInstr(), loc);
       if (var.getIndexAttr())

tools/clang/test/CodeGenSPIRV_Lit/vk.location.double4.error.hlsl

@@ -0,0 +1,12 @@
+// RUN: not %dxc -T ps_6_0 -E main -fcgl  %s -spirv 2>&1 | FileCheck %s
+
+struct PSInput
+{
+    [[vk::location(0)]] double4 data1 : COLOR0; // double4 consumes 2 Locations (0 and 1)
+    [[vk::location(1)]] double4 data2 : COLOR1;
+    // CHECK: error: stage input location #1 already consumed by semantic 'COLOR0'
+};
+
+void main(PSInput input) : SV_TARGET
+{
+}

tools/clang/test/CodeGenSPIRV_Lit/vk.location.reassigned-for-input.error.hlsl

@@ -0,0 +1,13 @@
+// RUN: not %dxc -T vs_6_0 -E main -fcgl -spirv %s 2>&1 | FileCheck %s
+
+struct S {
+    [[vk::location(3)]] float a : A;
+// CHECK: error: stage input location #3 already consumed by semantic 'M'
+};
+
+float main(
+    [[vk::location(3)]]     float m : M,
+    S s
+) : R {
+    return 1.0;
+}

tools/clang/test/CodeGenSPIRV_Lit/vk.location.reassigned-for-output.error.hlsl

@@ -0,0 +1,14 @@
+// RUN: not %dxc -T vs_6_0 -E main -fcgl -spirv %s 2>&1 | FileCheck %s
+
+struct T {
+    [[vk::location(3)]] float i : A;
+// CHECK: error: stage output location #3 already consumed by semantic 'M'
+};
+
+[[vk::location(3)]]
+float main(
+    [[vk::location(3)]]     float m : M,
+    out T t
+) : R {
+    return 1.0;
+}

tools/clang/test/CodeGenSPIRV_Lit/vk.location.reassigned.error.hlsl

@@ -0,0 +1,12 @@
+// RUN: not %dxc -T vs_6_0 -E main -fcgl  %s -spirv 2>&1 | FileCheck %s
+
+[[vk::location(3)]]
+float main(
+    [[vk::location(3)]]     float m : M,
+    [[vk::location(3)]]     float n : N,
+// CHECK: error: stage input location #3 already consumed by semantic 'M'
+    [[vk::location(3)]] out float x : X
+// CHECK: error: stage output location #3 already consumed by semantic 'M'
+) : R {
+    return 1.0;
+}

tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -2041,63 +2041,6 @@ TEST_F(FileTest, VulkanLocationPartiallyAssigned) {
   runFileTest("vk.location.mixed.hlsl", Expect::Failure);
 }
 
-std::string getStageLocationReassignTestShader(const std::string &typeDef,
-                                               const std::string &stageVar,
-                                               const std::string &check) {
-  const std::string command(R"(// RUN: %dxc -T vs_6_0 -E main)");
-  const std::string shader = command + typeDef + R"(
-[[vk::location(3)]]                   // first use
-float main(
-    [[vk::location(3)]]     float m : M,  // first use
-)" + stageVar + R"(
-) : R {
-    return 1.0;
-}
-)" + check;
-  return shader;
-}
-
-TEST_F(FileTest, VulkanLocationReassigned) {
-  runCodeTest(getStageLocationReassignTestShader(R"()",
-                                                 R"(
-    [[vk::location(3)]]     float n : N,  // error
-    [[vk::location(3)]] out float x : X   // error
-)",
-                                                 R"(
-// CHECK: error: stage input location #3 already assigned
-// CHECK: error: stage output location #3 already assigned
-)"),
-              Expect::Failure);
-}
-
-TEST_F(FileTest, VulkanLocationReassignedForInputVar) {
-  runCodeTest(
-      getStageLocationReassignTestShader(
-          R"(
-struct S {
-    [[vk::location(3)]] float a : A;  // error
-    [[vk::location(3)]] float b : B;  // error
-};
-)",
-          R"(S s)",
-          R"(// CHECK: error: stage input location #3 already assigned)"),
-      Expect::Failure);
-}
-
-TEST_F(FileTest, VulkanLocationReassignedForOutputVar) {
-  runCodeTest(getStageLocationReassignTestShader(
-                  R"(
-struct T {
-    [[vk::location(3)]] float i : A;  // error
-    [[vk::location(3)]] float j : B;  // error
-};
-)",
-                  R"(out T t)",
-                  R"(
-// CHECK: error: stage output location #3 already assigned
-)"),
-              Expect::Failure);
-}
 TEST_F(FileTest, StageVariableDuplicatedLocation) {
   runFileTest("semantic.duplicated-location.hlsl", Expect::Failure);
 }