Browse Source

[spirv] Emit note for the intial use location of bindings (#811)

This makes it easier for the devs to see with which a binding
numbers is conflicting.
Lei Zhang 7 years ago
parent
commit
21f16a1eaf

+ 16 - 6
tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -507,16 +507,20 @@ private:
 class BindingSet {
 public:
   /// Tries to use the given set and binding number. Returns true if possible,
-  /// false otherwise.
+  /// false otherwise and writes the source location of where the binding number
+  /// is used to *usedLoc.
   bool tryToUseBinding(uint32_t binding, uint32_t set,
-                       ResourceVar::Category category) {
+                       ResourceVar::Category category, SourceLocation tryLoc,
+                       SourceLocation *usedLoc) {
     const auto cat = static_cast<uint32_t>(category);
     // Note that we will create the entry for binding in bindings[set] here.
     // But that should not have bad effects since it defaults to zero.
     if ((usedBindings[set][binding] & cat) == 0) {
       usedBindings[set][binding] |= cat;
+      whereUsed[set][binding] = tryLoc;
       return true;
     }
+    *usedLoc = whereUsed[set][binding];
     return false;
   }
 
@@ -533,6 +537,8 @@ public:
 private:
   ///< set number -> (binding number -> resource category)
   llvm::DenseMap<uint32_t, llvm::DenseMap<uint32_t, uint32_t>> usedBindings;
+  ///< set number -> (binding number -> source location)
+  llvm::DenseMap<uint32_t, llvm::DenseMap<uint32_t, SourceLocation>> whereUsed;
   ///< set number -> next available binding number
   llvm::DenseMap<uint32_t, uint32_t> nextBindings;
 };
@@ -686,7 +692,7 @@ private:
   uint32_t masterShift; /// Shift amount applies to all sets.
   llvm::DenseMap<uint32_t, uint32_t> perSetShift;
 };
-}
+} // namespace
 
 bool DeclResultIdMapper::decorateResourceBindings() {
   // For normal resource, we support 3 approaches of setting binding numbers:
@@ -713,14 +719,18 @@ bool DeclResultIdMapper::decorateResourceBindings() {
   // Tries to decorate the given varId of the given category with set number
   // setNo, binding number bindingNo. Emits error on failure.
   const auto tryToDecorate = [this, &bindingSet, &noError](
-      const uint32_t varId, const uint32_t setNo, const uint32_t bindingNo,
-      const ResourceVar::Category cat, SourceLocation loc) {
-    if (bindingSet.tryToUseBinding(bindingNo, setNo, cat)) {
+                                 const uint32_t varId, const uint32_t setNo,
+                                 const uint32_t bindingNo,
+                                 const ResourceVar::Category cat,
+                                 SourceLocation loc) {
+    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)
           << bindingNo << setNo;
+      emitNote("binding number previously assigned here", prevUseLoc);
       noError = false;
     }
   };

+ 10 - 1
tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -291,6 +291,15 @@ private:
     return diags.Report(loc, diagId);
   }
 
+  /// \brief Wrapper method to create a note message and report it
+  /// in the diagnostic engine associated with this consumer.
+  template <unsigned N>
+  DiagnosticBuilder emitNote(const char (&message)[N], SourceLocation loc) {
+    const auto diagId =
+        diags.getCustomDiagID(clang::DiagnosticsEngine::Note, message);
+    return diags.Report(loc, diagId);
+  }
+
   /// \brief Checks whether some semantic is used more than once and returns
   /// true if no such cases. Returns false otherwise.
   bool checkSemanticDuplication(bool forInput);
@@ -431,4 +440,4 @@ bool DeclResultIdMapper::isInputStorageClass(const StageVar &v) {
 } // end namespace spirv
 } // end namespace clang
 
-#endif
+#endif

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

@@ -23,6 +23,7 @@ float4 main() : SV_Target {
 }
 
 //CHECK: :10:30: error: resource binding #2 in descriptor set #0 already assigned
+//CHECK:   :7:3: note: binding number previously assigned here
 
 //CHECK: :13:29: error: resource binding #2 in descriptor set #0 already assigned
 

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

@@ -36,6 +36,7 @@ float4 main() : SV_Target {
 // 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: :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

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

@@ -23,6 +23,7 @@ float4 main() : SV_Target {
 }
 
 // CHECK: :10:36: error: resource binding #0 in descriptor set #0 already assigned
+// 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