Address Comments

Author: V-FEXrt

llvm/include/llvm/Analysis/DXILResource.h

@@ -439,8 +439,12 @@ class DXILResourceMap {
   unsigned FirstCBuffer = 0;
   unsigned FirstSampler = 0;
 
-  /// Populate the map given the resource binding calls in the given module.
+  /// Populate all the resource instance data.
   void populate(Module &M, DXILResourceTypeMap &DRTM);
+  /// Populate the map given the resource binding calls in the given module.
+  void populateResourceInfos(Module &M, DXILResourceTypeMap &DRTM);
+  /// Analyze and populate the directions of the resource counters.
+  void populateCounterDirections(Module &M);
 
 public:
   using iterator = SmallVector<dxil::ResourceInfo>::iterator;

llvm/lib/Analysis/DXILResource.cpp

@@ -701,7 +701,8 @@ static bool isUpdateCounterIntrinsic(Function &F) {
   return F.getIntrinsicID() == Intrinsic::dx_resource_updatecounter;
 }
 
-void DXILResourceMap::populate(Module &M, DXILResourceTypeMap &DRTM) {
+void DXILResourceMap::populateResourceInfos(Module &M,
+                                            DXILResourceTypeMap &DRTM) {
   SmallVector<std::tuple<CallInst *, ResourceInfo, ResourceTypeInfo>> CIToInfos;
 
   for (Function &F : M.functions()) {
@@ -778,7 +779,9 @@ void DXILResourceMap::populate(Module &M, DXILResourceTypeMap &DRTM) {
     // Adjust the resource binding to use the next ID.
     RI.setBindingID(NextID++);
   }
+}
 
+void DXILResourceMap::populateCounterDirections(Module &M) {
   for (Function &F : M.functions()) {
     if (!isUpdateCounterIntrinsic(F))
       continue;
@@ -806,16 +809,20 @@ void DXILResourceMap::populate(Module &M, DXILResourceTypeMap &DRTM) {
       Value *HandleArg = CI->getArgOperand(0);
       SmallVector<ResourceInfo *> RBInfos = findByUse(HandleArg);
       for (ResourceInfo *RBInfo : RBInfos) {
-        if (RBInfo->CounterDirection == ResourceCounterDirection::Unknown ||
-            RBInfo->CounterDirection == Direction)
+        if (RBInfo->CounterDirection == ResourceCounterDirection::Unknown)
           RBInfo->CounterDirection = Direction;
-        else
+        else if (RBInfo->CounterDirection != Direction)
           RBInfo->CounterDirection = ResourceCounterDirection::Invalid;
       }
     }
   }
 }
 
+void DXILResourceMap::populate(Module &M, DXILResourceTypeMap &DRTM) {
+  populateResourceInfos(M, DRTM);
+  populateCounterDirections(M);
+}
+
 void DXILResourceMap::print(raw_ostream &OS, DXILResourceTypeMap &DRTM,
                             const DataLayout &DL) const {
   for (unsigned I = 0, E = Infos.size(); I != E; ++I) {
@@ -849,9 +856,8 @@ SmallVector<dxil::ResourceInfo *> DXILResourceMap::findByUse(const Value *Key) {
   // Found the create, return the binding
   case Intrinsic::dx_resource_handlefrombinding: {
     auto Pos = CallMap.find(CI);
-    ResourceInfo *It = &Infos[Pos->second];
-    assert(It != Infos.end() && "HandleFromBinding must be in resource map");
-    return {It};
+    assert(Pos != CallMap.end() && "HandleFromBinding must be in resource map");
+    return {&Infos[Pos->second]};
   }
   default:
     break;

llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp

@@ -293,9 +293,8 @@ define void @main() {
   DXILResourceMap &DRM = MAM->getResult<DXILResourceAnalysis>(*M);
 
   for (const Function &F : M->functions()) {
-    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding) {
+    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding)
       continue;
-    }
 
     for (const User *U : F.users()) {
       const CallInst *CI = cast<CallInst>(U);
@@ -324,9 +323,8 @@ define void @main() {
   DXILResourceMap &DRM = MAM->getResult<DXILResourceAnalysis>(*M);
 
   for (const Function &F : M->functions()) {
-    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding) {
+    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding)
       continue;
-    }
 
     for (const User *U : F.users()) {
       const CallInst *CI = cast<CallInst>(U);
@@ -352,9 +350,8 @@ define void @main() {
   DXILResourceMap &DRM = MAM->getResult<DXILResourceAnalysis>(*M);
 
   for (const Function &F : M->functions()) {
-    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding) {
+    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding)
       continue;
-    }
 
     for (const User *U : F.users()) {
       const CallInst *CI = cast<CallInst>(U);
@@ -387,15 +384,16 @@ define void @main() {
   ResourceCounterDirection *Dir = Dirs;
 
   for (const Function &F : M->functions()) {
-    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding) {
+    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding)
       continue;
-    }
 
+    uint32_t ExpectedDirsIndex = 0;
     for (const User *U : F.users()) {
       const CallInst *CI = cast<CallInst>(U);
       const auto *const Binding = DRM.find(CI);
-      ASSERT_EQ(Binding->CounterDirection, *Dir);
-      Dir++;
+      ASSERT_TRUE(ExpectedDirsIndex < 2);
+      ASSERT_EQ(Binding->CounterDirection, Dir[ExpectedDirsIndex]);
+      ExpectedDirsIndex++;
     }
   }
 }
@@ -417,9 +415,8 @@ define void @main() {
   DXILResourceMap &DRM = MAM->getResult<DXILResourceAnalysis>(*M);
 
   for (const Function &F : M->functions()) {
-    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding) {
+    if (F.getIntrinsicID() != Intrinsic::dx_resource_handlefrombinding)
       continue;
-    }
 
     for (const User *U : F.users()) {
       const CallInst *CI = cast<CallInst>(U);