Pare back to analysis only

Author: V-FEXrt

llvm/lib/Analysis/DXILResource.cpp

@@ -11,7 +11,6 @@
 #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"
@@ -22,7 +21,6 @@
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/FormatVariadic.h"
 #include <algorithm>
-#include <iterator>
 
 #define DEBUG_TYPE "dxil-resource"
 
@@ -832,11 +830,6 @@ static bool isUpdateCounterIntrinsic(Function &F) {
 }
 
 void DXILResourceCounterDirectionMap::populate(Module &M, DXILBindingMap &DBM) {
-  SmallVector<
-      std::tuple<const dxil::ResourceBindingInfo *, ResourceCounterDirection,
-                 const Function *, const CallInst *>>
-      DiagCounterDirs;
-
   for (Function &F : M.functions()) {
     if (!isUpdateCounterIntrinsic(F))
       continue;
@@ -863,82 +856,42 @@ void DXILResourceCounterDirectionMap::populate(Module &M, DXILBindingMap &DBM) {
       SmallVector<const dxil::ResourceBindingInfo *> RBInfos =
           DBM.findByUse(HandleArg);
       for (const dxil::ResourceBindingInfo *RBInfo : RBInfos)
-        DiagCounterDirs.emplace_back(RBInfo, Direction, &F, CI);
+        CounterDirections.emplace_back(RBInfo, Direction);
     }
   }
 
   // Sort by the Binding and Direction for fast lookup
-  std::sort(DiagCounterDirs.begin(), DiagCounterDirs.end(),
-            [](const auto &LHS, const auto &RHS) {
-              const auto L =
-                  std::pair{std::get<const dxil::ResourceBindingInfo *>(LHS),
-                            std::get<ResourceCounterDirection>(LHS)};
-              const auto R =
-                  std::pair{std::get<const dxil::ResourceBindingInfo *>(RHS),
-                            std::get<ResourceCounterDirection>(RHS)};
-              return L < R;
-            });
+  std::sort(CounterDirections.begin(), CounterDirections.end());
 
-  {
-    auto *SpanStart = DiagCounterDirs.begin();
-    auto *SpanEnd = SpanStart;
-    auto *ItEnd = DiagCounterDirs.end();
-
-    while (SpanStart < ItEnd) {
-      while (SpanEnd < ItEnd &&
-             std::get<const dxil::ResourceBindingInfo *>(*SpanStart) ==
-                 std::get<const dxil::ResourceBindingInfo *>(*SpanEnd))
-        SpanEnd++;
-
-      // SpanStart : SpanEnd-1 are the same binding. Its an error if they aren't
-      // all the same direction
-      ResourceCounterDirection ResourceDir =
-          std::get<ResourceCounterDirection>(*SpanStart);
-      bool ValidUse = true;
-      for (auto *CheckIt = SpanStart; CheckIt < SpanEnd; ++CheckIt) {
-        if (ResourceDir != std::get<ResourceCounterDirection>(*CheckIt)) {
-          ValidUse = false;
-          break;
-        }
-      }
+  // Remove the duplicate entries. Since direction is considered for equality
+  // a unique resource with more than one direction will not be deduped.
+  auto UniqueEnd =
+      std::unique(CounterDirections.begin(), CounterDirections.end());
 
-      // Update the direction and raise a diag when an invalid use is detected
-      for (auto *RaiseIt = SpanStart; !ValidUse && RaiseIt < SpanEnd;
-           ++RaiseIt) {
-        std::get<ResourceCounterDirection>(*RaiseIt) =
+  // 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.
+  // Mark all those entries as invalid.
+  {
+    auto DupFirst = CounterDirections.begin();
+    auto DupNext = DupFirst + 1;
+    auto DupLast = UniqueEnd;
+    for (; DupFirst < DupLast && DupNext < DupLast; ++DupFirst, ++DupNext) {
+      if (std::get<const dxil::ResourceBindingInfo *>(*DupFirst) ==
+          std::get<const dxil::ResourceBindingInfo *>(*DupNext)) {
+        std::get<ResourceCounterDirection>(*DupFirst) =
+            ResourceCounterDirection::Invalid;
+        std::get<ResourceCounterDirection>(*DupNext) =
             ResourceCounterDirection::Invalid;
-
-        StringRef Message =
-            "RWStructuredBuffers may increment or decrement their "
-            "counters, but not both.";
-        const Function *F = std::get<const Function *>(*RaiseIt);
-        const CallInst *CI = std::get<const CallInst *>(*RaiseIt);
-        M.getContext().diagnose(
-            DiagnosticInfoGenericWithLoc(Message, *F, CI->getDebugLoc()));
       }
-
-      SpanStart = SpanEnd;
     }
   }
 
-  // Remove the duplicate entries. All entries with the same resource have the
-  // same direction so it can be ignored.
-  auto *const UniqueEnd =
-      std::unique(DiagCounterDirs.begin(), DiagCounterDirs.end(),
-                  [](const auto &LHS, const auto &RHS) {
-                    return std::get<const dxil::ResourceBindingInfo *>(LHS) ==
-                           std::get<const dxil::ResourceBindingInfo *>(RHS);
-                  });
-
-  // Copy the results into the final location
-  CounterDirections.clear();
-  CounterDirections.reserve(UniqueEnd - DiagCounterDirs.begin());
-  std::transform(DiagCounterDirs.begin(), UniqueEnd,
-                 std::back_inserter(CounterDirections), [](const auto &Item) {
-                   return std::pair{
-                       std::get<const dxil::ResourceBindingInfo *>(Item),
-                       std::get<ResourceCounterDirection>(Item)};
-                 });
+  // Remove the duplicate entries again now that each duplicate resource has the
+  // same direction for each entry leaving one entry per RBI
+  UniqueEnd = std::unique(CounterDirections.begin(), UniqueEnd);
+
+  // Actually erase the invalidated items
+  CounterDirections.erase(UniqueEnd, CounterDirections.end());
 }
 
 ResourceCounterDirection DXILResourceCounterDirectionMap::operator[](

llvm/lib/Target/DirectX/DXILOpLowering.cpp

@@ -859,14 +859,6 @@ PreservedAnalyses DXILOpLowering::run(Module &M, ModuleAnalysisManager &MAM) {
   DXILBindingMap &DBM = MAM.getResult<DXILResourceBindingAnalysis>(M);
   DXILResourceTypeMap &DRTM = MAM.getResult<DXILResourceTypeAnalysis>(M);
 
-  // TODO: This needs to be called even though its not currently being used in
-  // order for tests to pass. It will eventually need to be used as part of
-  // https://github.com/llvm/llvm-project/issues/125126 which will have the same
-  // effect. Until then discard the result.
-  DXILResourceCounterDirectionMap &DRCDM =
-      MAM.getResult<DXILResourceCounterDirectionAnalysis>(M);
-  (void)DRCDM;
-
   bool MadeChanges = OpLowerer(M, DBM, DRTM).lowerIntrinsics();
   if (!MadeChanges)
     return PreservedAnalyses::all();

llvm/test/CodeGen/DirectX/resource_counter_error.ll

@@ -371,8 +371,8 @@ define void @main() {
   }
 }
 
-// Test that multiple different resources with unique incs/decs don't
-// raise a Diagnostic and can be individually checked for direction
+// Test that multiple different resources with unique incs/decs aren't
+// marked invalid
 TEST_F(UniqueResourceFromUseTest, TestResourceCounterMultiple) {
   StringRef Assembly = R"(
 define void @main() {
@@ -409,4 +409,35 @@ define void @main() {
   }
 }
 
+// Test that single different resources with unique incs/decs is marked invalid
+TEST_F(UniqueResourceFromUseTest, TestResourceCounterInvalid) {
+  StringRef Assembly = R"(
+define void @main() {
+entry:
+  %handle = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding(i32 1, i32 2, i32 3, i32 4, i1 false)
+  call i32 @llvm.dx.resource.updatecounter(target("dx.RawBuffer", float, 1, 0) %handle, i8 -1)
+  call i32 @llvm.dx.resource.updatecounter(target("dx.RawBuffer", float, 1, 0) %handle, i8 1)
+  ret void
+}
+  )";
+
+  auto M = parseAsm(Assembly);
+
+  const DXILBindingMap &DBM = MAM->getResult<DXILResourceBindingAnalysis>(*M);
+  const DXILResourceCounterDirectionMap &DCDM =
+      MAM->getResult<DXILResourceCounterDirectionAnalysis>(*M);
+
+  for (const Function &F : M->functions()) {
+    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding) {
+      continue;
+    }
+
+    for (const User *U : F.users()) {
+      const CallInst *CI = cast<CallInst>(U);
+      const auto *const Binding = DBM.find(CI);
+      ASSERT_EQ(DCDM[*Binding], ResourceCounterDirection::Invalid);
+    }
+  }
+}
+
 } // namespace