Add location to error message

Author: V-FEXrt

llvm/lib/Analysis/DXILResource.cpp

@@ -11,6 +11,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Instructions.h"
@@ -21,6 +22,7 @@
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/FormatVariadic.h"
 #include <algorithm>
+#include <iterator>
 
 #define DEBUG_TYPE "dxil-resource"
 
@@ -830,7 +832,8 @@ static bool isUpdateCounterIntrinsic(Function &F) {
 }
 
 void DXILResourceCounterDirectionMap::populate(Module &M, DXILBindingMap &DBM) {
-  CounterDirections.clear();
+  std::vector<std::tuple<dxil::ResourceBindingInfo, ResourceCounterDirection, const Function*, const CallInst*>>
+      DiagCounterDirs;
 
   for (Function &F : M.functions()) {
     if (!isUpdateCounterIntrinsic(F))
@@ -855,39 +858,59 @@ void DXILResourceCounterDirectionMap::populate(Module &M, DXILBindingMap &DBM) {
       Value *HandleArg = CI->getArgOperand(0);
       SmallVector<dxil::ResourceBindingInfo> RBInfos = DBM.findByUse(HandleArg);
       for (const dxil::ResourceBindingInfo RBInfo : RBInfos)
-        CounterDirections.emplace_back(RBInfo, Direction);
+        DiagCounterDirs.emplace_back(RBInfo, Direction, &F, CI);
     }
   }
 
   // An entry that is not in the map is considered unknown so its wasted
   // overhead and increased complexity to keep an entry explicitly marked
   // unknown
   const auto RemoveEnd = std::remove_if(
-      CounterDirections.begin(), CounterDirections.end(), [](const auto &Item) {
-        return Item.second == ResourceCounterDirection::Unknown;
+      DiagCounterDirs.begin(), DiagCounterDirs.end(), [](const auto &Item) {
+        return std::get<ResourceCounterDirection>(Item) == ResourceCounterDirection::Unknown;
       });
 
-  // Order for fast lookup
-  std::sort(CounterDirections.begin(), RemoveEnd);
+  // Sort by the Binding and Direction for fast lookup
+  std::sort(DiagCounterDirs.begin(), RemoveEnd, [](const auto &LHS, const auto &RHS) {
+    const auto L = std::pair{std::get<dxil::ResourceBindingInfo>(LHS), std::get<ResourceCounterDirection>(LHS)};
+    const auto R = std::pair{std::get<dxil::ResourceBindingInfo>(RHS), std::get<ResourceCounterDirection>(RHS)};
+    return L < R;
+  });
 
   // Remove the duplicate entries. Since direction is considered for equality
   // a unique resource with more than one direction will not be deduped.
-  const auto UniqueEnd = std::unique(CounterDirections.begin(), RemoveEnd);
+  const auto UniqueEnd = std::unique(DiagCounterDirs.begin(), RemoveEnd, [](const auto &LHS, const auto &RHS) {
+    const auto L = std::pair{std::get<dxil::ResourceBindingInfo>(LHS), std::get<ResourceCounterDirection>(LHS)};
+    const auto R = std::pair{std::get<dxil::ResourceBindingInfo>(RHS), std::get<ResourceCounterDirection>(RHS)};
+    return L == R;
+  });
 
   // Actually erase the items invalidated by remove_if + unique
-  CounterDirections.erase(UniqueEnd, CounterDirections.end());
+  DiagCounterDirs.erase(UniqueEnd, DiagCounterDirs.end());
 
   // If any duplicate entries still exist at this point then it must be a
   // resource that was both incremented and decremented which is not allowed.
   const auto DuplicateEntry = std::adjacent_find(
-      CounterDirections.begin(), CounterDirections.end(),
-      [](const auto &LHS, const auto &RHS) { return LHS.first == RHS.first; });
-  if (DuplicateEntry == CounterDirections.end())
+      DiagCounterDirs.begin(), DiagCounterDirs.end(),
+      [](const auto &LHS, const auto &RHS) {
+      return std::get<dxil::ResourceBindingInfo>(LHS) == std::get<dxil::ResourceBindingInfo>(RHS);
+  });
+
+  // Copy the results into the final vec
+  CounterDirections.clear();
+  CounterDirections.reserve(DiagCounterDirs.size());
+  std::transform(DiagCounterDirs.begin(), DiagCounterDirs.end(), std::back_inserter(CounterDirections), [](const auto &Item){
+    return std::pair{std::get<dxil::ResourceBindingInfo>(Item), std::get<ResourceCounterDirection>(Item)};
+  });
+
+  if (DuplicateEntry == DiagCounterDirs.end())
     return;
 
+  const Function* F = std::get<const Function*>(*DuplicateEntry);
+  const CallInst* CI = std::get<const CallInst*>(*DuplicateEntry);
   StringRef Message = "RWStructuredBuffers may increment or decrement their "
                       "counters, but not both.";
-  M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+  M.getContext().diagnose(DiagnosticInfoGenericWithLoc(Message, *F, CI->getDebugLoc()));
 }
 
 void DXILResourceCounterDirectionWrapperPass::getAnalysisUsage(

llvm/test/CodeGen/DirectX/resource_counter_error.ll

@@ -1,5 +1,5 @@
 ; RUN: not opt -S -passes='dxil-op-lower' -mtriple=dxil-pc-shadermodel6.3-library %s 2>&1 | FileCheck %s
-; CHECK:  RWStructuredBuffers may increment or decrement their counters, but not both.
+; CHECK: <unknown>:0:0: RWStructuredBuffers may increment or decrement their counters, but not both.
 
 define void @inc_and_dec() {
 entry:

llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp

@@ -281,6 +281,8 @@ declare target("dx.RawBuffer", float, 1, 0) @ind.func(target("dx.RawBuffer", flo
   }
 }
 
+// Test that several calls to decrement on the same resource don't raise a
+// Diagnositic and resolves to a single decrement entry
 TEST_F(UniqueResourceFromUseTest, TestResourceCounterDecrement) {
   StringRef Assembly = R"(
 define void @main() {
@@ -322,6 +324,8 @@ declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
   }
 }
 
+// Test that several calls to increment on the same resource don't raise a
+// Diagnositic and resolves to a single increment entry
 TEST_F(UniqueResourceFromUseTest, TestResourceCounterIncrement) {
   StringRef Assembly = R"(
 define void @main() {
@@ -363,6 +367,8 @@ declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
   }
 }
 
+// Test that looking up a resource that doesn't have the counter updated
+// resoves to unknown
 TEST_F(UniqueResourceFromUseTest, TestResourceCounterUnknown) {
   StringRef Assembly = R"(
 define void @main() {
@@ -401,6 +407,8 @@ declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
   }
 }
 
+// Test that multiple different resources with unique incs/decs don't
+// raise a Diagnostic and can be individually checked for direction
 TEST_F(UniqueResourceFromUseTest, TestResourceCounterMultiple) {
   StringRef Assembly = R"(
 define void @main() {