[SPIR-V] Block semantic annotation reuse for input (#6396)

The MSDN spec is not very clear regarding input parameter aliasing, BUT DXIL & MS agrees (See #3737) using the same semantic annotation twice should be disallowed. DXIL currently disallows it for most, and due to a bug, allows some compute related semantics to be aliased. SPIR-V did allowed aliasing, but it was a implemented as a hack causing typing issues if the type changed between the 2 parameters using the same semantic annotation. To align more closely with DXIL, and the "spec", we are now disallowing all semantic attribute reuse for input parameters. Fixes #3737 Signed-off-by: Nathan Gauër <[email protected]>
microsoft · Mar 8, 2024 · 1f162e2 · 1f162e2
1 parent 263a773
commit 1f162e2
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 33 deletions.
diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp
@@ -1836,7 +1836,6 @@ DeclResultIdMapper::collectStageVars(SpirvFunction *entryPoint) const {
   for (auto var : glPerVertex.getStageOutVars())
     vars.push_back(var);
 
-  llvm::DenseSet<SpirvInstruction *> seenVars;
   for (const auto &var : stageVars) {
     // We must collect stage variables that are included in entryPoint and stage
     // variables that are not included in any specific entryPoint i.e.,
@@ -1847,10 +1846,7 @@ DeclResultIdMapper::collectStageVars(SpirvFunction *entryPoint) const {
     auto *instr = var.getSpirvInstr();
     if (instr->getStorageClass() == spv::StorageClass::Private)
       continue;
-    if (seenVars.count(instr) == 0) {
-      vars.push_back(instr);
-      seenVars.insert(instr);
-    }
+    vars.push_back(instr);
   }
 
   return vars;
@@ -2043,23 +2039,22 @@ bool DeclResultIdMapper::checkSemanticDuplication(bool forInput) {
       continue;
     }
 
-    // Allow builtin variables to alias each other. We already have uniqify
-    // mechanism in SpirvBuilder.
-    if (var.isSpirvBuitin())
-      continue;
-
     if (forInput && var.getSigPoint()->IsInput()) {
       bool insertionSuccess = insertSeenSemanticsForEntryPointIfNotExist(
           &seenSemanticsForEntryPoints, var.getEntryPoint(), s);
       if (!insertionSuccess) {
-        emitError("input semantic '%0' used more than once", {}) << s;
+        emitError("input semantic '%0' used more than once",
+                  var.getSemanticInfo().loc)
+            << s;
         success = false;
       }
     } else if (!forInput && var.getSigPoint()->IsOutput()) {
       bool insertionSuccess = insertSeenSemanticsForEntryPointIfNotExist(
           &seenSemanticsForEntryPoints, var.getEntryPoint(), s);
       if (!insertionSuccess) {
-        emitError("output semantic '%0' used more than once", {}) << s;
+        emitError("output semantic '%0' used more than once",
+                  var.getSemanticInfo().loc)
+            << s;
         success = false;
       }
     }

diff --git a/tools/clang/test/CodeGenSPIRV/decoration.unique.hlsl b/tools/clang/test/CodeGenSPIRV/decoration.unique.hlsl
diff --git a/tools/clang/test/CodeGenSPIRV/spirv.interface.alias-builtin.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.interface.alias-builtin.hlsl
@@ -1,21 +1,12 @@
-// RUN: %dxc -T ps_6_0 -E main -fcgl  %s -spirv | FileCheck %s
-
-// CHECK: OpEntryPoint Fragment %main "main" %gl_FragCoord %out_var_SV_Target
-
-// CHECK:     %gl_FragCoord = OpVariable %_ptr_Input_v4float Input
-// CHECK-NOT:      {{%[0-9]+}} = OpVariable %_ptr_Input_v4float Input
+// RUN: not %dxc -T ps_6_0 -E main %s -spirv 2>&1 | FileCheck %s
 
 struct PSInput {
   float4 a : SV_Position;
   float4 b : SV_Position;
+// CHECK: :5:14: error: input semantic 'SV_Position' used more than once
 };
 
-// CHECK:      [[a:%[0-9]+]] = OpLoad %v4float %gl_FragCoord
-// CHECK-NEXT: [[b:%[0-9]+]] = OpLoad %v4float %gl_FragCoord
-// CHECK-NEXT:   {{%[0-9]+}} = OpCompositeConstruct %PSInput [[a]] [[b]]
-
 float4 main(PSInput input) : SV_Target
 {
 	return input.a + input.b;
 }
-