Browse Source

[spirv] Emit warnings for the same binding number (#816)

This is to enable certain workflows that reflection is used to
change the binding numbers after codegen.
Lei Zhang 7 years ago
parent
commit
ddf64b3258

+ 7 - 8
tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -716,23 +716,22 @@ bool DeclResultIdMapper::decorateResourceBindings() {
   BindingSet bindingSet;
   BindingSet bindingSet;
   bool noError = true;
   bool noError = true;
 
 
-  // Tries to decorate the given varId of the given category with set number
-  // setNo, binding number bindingNo. Emits error on failure.
+  // Decorates the given varId of the given category with set number
+  // setNo, binding number bindingNo. Emits warning if overlap.
   const auto tryToDecorate = [this, &bindingSet, &noError](
   const auto tryToDecorate = [this, &bindingSet, &noError](
                                  const uint32_t varId, const uint32_t setNo,
                                  const uint32_t varId, const uint32_t setNo,
                                  const uint32_t bindingNo,
                                  const uint32_t bindingNo,
                                  const ResourceVar::Category cat,
                                  const ResourceVar::Category cat,
                                  SourceLocation loc) {
                                  SourceLocation loc) {
     SourceLocation prevUseLoc;
     SourceLocation prevUseLoc;
-    if (bindingSet.tryToUseBinding(bindingNo, setNo, cat, loc, &prevUseLoc)) {
-      theBuilder.decorateDSetBinding(varId, setNo, bindingNo);
-    } else {
-      emitError("resource binding #%0 in descriptor set #%1 already assigned",
-                loc)
+    if (!bindingSet.tryToUseBinding(bindingNo, setNo, cat, loc, &prevUseLoc)) {
+      emitWarning("resource binding #%0 in descriptor set #%1 already assigned",
+                  loc)
           << bindingNo << setNo;
           << bindingNo << setNo;
       emitNote("binding number previously assigned here", prevUseLoc);
       emitNote("binding number previously assigned here", prevUseLoc);
-      noError = false;
+      // noError = false;
     }
     }
+    theBuilder.decorateDSetBinding(varId, setNo, bindingNo);
   };
   };
 
 
   for (const auto &var : resourceVars) {
   for (const auto &var : resourceVars) {

+ 9 - 0
tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -291,6 +291,15 @@ private:
     return diags.Report(loc, diagId);
     return diags.Report(loc, diagId);
   }
   }
 
 
+  /// \brief Wrapper method to create a warning message and report it
+  /// in the diagnostic engine associated with this consumer.
+  template <unsigned N>
+  DiagnosticBuilder emitWarning(const char (&message)[N], SourceLocation loc) {
+    const auto diagId =
+        diags.getCustomDiagID(clang::DiagnosticsEngine::Warning, message);
+    return diags.Report(loc, diagId);
+  }
+
   /// \brief Wrapper method to create a note message and report it
   /// \brief Wrapper method to create a note message and report it
   /// in the diagnostic engine associated with this consumer.
   /// in the diagnostic engine associated with this consumer.
   template <unsigned N>
   template <unsigned N>

+ 3 - 3
tools/clang/test/CodeGenSPIRV/vk.binding.cl.error.hlsl

@@ -22,9 +22,9 @@ float4 main() : SV_Target {
     return cbuffer1.f;
     return cbuffer1.f;
 }
 }
 
 
-//CHECK: :10:30: error: resource binding #2 in descriptor set #0 already assigned
+//CHECK: :10:30: warning: resource binding #2 in descriptor set #0 already assigned
 //CHECK:   :7:3: note: binding number previously assigned here
 //CHECK:   :7:3: note: binding number previously assigned here
 
 
-//CHECK: :13:29: error: resource binding #2 in descriptor set #0 already assigned
+//CHECK: :13:29: warning: resource binding #2 in descriptor set #0 already assigned
 
 
-//CHECK: :19:30: error: resource binding #3 in descriptor set #0 already assigned
+//CHECK: :19:30: warning: resource binding #3 in descriptor set #0 already assigned

+ 6 - 6
tools/clang/test/CodeGenSPIRV/vk.binding.explicit.error.hlsl

@@ -33,10 +33,10 @@ float4 main() : SV_Target {
     return 1.0;
     return 1.0;
 }
 }
 
 
-// CHECK-NOT:  :9:{{%\d+}}: error: resource binding #1 in descriptor set #0 already assigned
-// CHECK-NOT: :12:{{%\d+}}: error: resource binding #3 in descriptor set #1 already assigned
-// CHECK: :15:3: error: resource binding #3 in descriptor set #1 already assigned
+// CHECK-NOT:  :9:{{%\d+}}: warning: resource binding #1 in descriptor set #0 already assigned
+// CHECK-NOT: :12:{{%\d+}}: warning: resource binding #3 in descriptor set #1 already assigned
+// CHECK: :15:3: warning: resource binding #3 in descriptor set #1 already assigned
 // CHECK: :12:3: note: binding number previously assigned here
 // CHECK: :12:3: note: binding number previously assigned here
-// CHECK: :18:3: error: resource binding #1 in descriptor set #0 already assigned
-// CHECK: :26:3: error: resource binding #5 in descriptor set #0 already assigned
-// CHECK: :29:3: error: resource binding #5 in descriptor set #0 already assigned
+// CHECK: :18:3: warning: resource binding #1 in descriptor set #0 already assigned
+// CHECK: :26:3: warning: resource binding #5 in descriptor set #0 already assigned
+// CHECK: :29:3: warning: resource binding #5 in descriptor set #0 already assigned

+ 4 - 4
tools/clang/test/CodeGenSPIRV/vk.binding.register.error.hlsl

@@ -22,8 +22,8 @@ float4 main() : SV_Target {
     return 1.0;
     return 1.0;
 }
 }
 
 
-// CHECK: :10:36: error: resource binding #0 in descriptor set #0 already assigned
+// CHECK: :10:36: warning: resource binding #0 in descriptor set #0 already assigned
 // CHECK:  :7:36: note: binding number previously assigned here
 // CHECK:  :7:36: note: binding number previously assigned here
-// CHECK: :11:36: error: resource binding #0 in descriptor set #1 already assigned
-// CHECK-NOT: :15:{{%\d+}}: error: resource binding #5 in descriptor set #1 already assigned
-// CHECK-NOT: :18:{{%\d+}}: error: resource binding #6 in descriptor set #6 already assigned
+// CHECK: :11:36: warning: resource binding #0 in descriptor set #1 already assigned
+// CHECK-NOT: :15:{{%\d+}}: warning: resource binding #5 in descriptor set #1 already assigned
+// CHECK-NOT: :18:{{%\d+}}: warning: resource binding #6 in descriptor set #6 already assigned

+ 3 - 3
tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -912,13 +912,13 @@ TEST_F(FileTest, VulkanRegisterBindingShift) {
   runFileTest("vk.binding.cl.hlsl");
   runFileTest("vk.binding.cl.hlsl");
 }
 }
 TEST_F(FileTest, VulkanExplicitBindingReassigned) {
 TEST_F(FileTest, VulkanExplicitBindingReassigned) {
-  runFileTest("vk.binding.explicit.error.hlsl", FileTest::Expect::Failure);
+  runFileTest("vk.binding.explicit.error.hlsl", FileTest::Expect::Warning);
 }
 }
 TEST_F(FileTest, VulkanRegisterBindingReassigned) {
 TEST_F(FileTest, VulkanRegisterBindingReassigned) {
-  runFileTest("vk.binding.register.error.hlsl", FileTest::Expect::Failure);
+  runFileTest("vk.binding.register.error.hlsl", FileTest::Expect::Warning);
 }
 }
 TEST_F(FileTest, VulkanRegisterBindingShiftReassigned) {
 TEST_F(FileTest, VulkanRegisterBindingShiftReassigned) {
-  runFileTest("vk.binding.cl.error.hlsl", FileTest::Expect::Failure);
+  runFileTest("vk.binding.cl.error.hlsl", FileTest::Expect::Warning);
 }
 }
 TEST_F(FileTest, VulkanStructuredBufferCounter) {
 TEST_F(FileTest, VulkanStructuredBufferCounter) {
   // [[vk::counter_binding()]] for RWStructuredBuffer, AppendStructuredBuffer,
   // [[vk::counter_binding()]] for RWStructuredBuffer, AppendStructuredBuffer,