address comments

Author: V-FEXrt

llvm/include/llvm/Analysis/DXILResource.h

@@ -446,8 +446,11 @@ class DXILBindingMap {
     return Pos == CallMap.end() ? Infos.end() : (Infos.begin() + Pos->second);
   }
 
-  // Resoloves the use of a resource handle into the unique description of that
-  // resource by deduping calls to create.
+  /// Resolves a resource handle into a vector of ResourceBindingInfos that
+  /// represent the possible unique creations of the handle. Certain cases are
+  /// ambiguous so mulitple creation points may be returned. The resulting
+  /// ResourceBindingInfo can be used to depuplicate unique handles that
+  /// reference the same resource
   SmallVector<dxil::ResourceBindingInfo> findByUse(const Value *Key) const;
 
   const_iterator find(const CallInst *Key) const {

llvm/lib/Analysis/DXILResource.cpp

@@ -772,8 +772,7 @@ void DXILBindingMap::print(raw_ostream &OS, DXILResourceTypeMap &DRTM,
 
 SmallVector<dxil::ResourceBindingInfo>
 DXILBindingMap::findByUse(const Value *Key) const {
-  const PHINode *Phi = dyn_cast<PHINode>(Key);
-  if (Phi) {
+  if (const PHINode *Phi = dyn_cast<PHINode>(Key)) {
     SmallVector<dxil::ResourceBindingInfo> Children;
     for (const Value *V : Phi->operands()) {
       Children.append(findByUse(V));
@@ -782,9 +781,8 @@ DXILBindingMap::findByUse(const Value *Key) const {
   }
 
   const CallInst *CI = dyn_cast<CallInst>(Key);
-  if (!CI) {
+  if (!CI)
     return {};
-  }
 
   const Type *UseType = CI->getType();
 
@@ -794,9 +792,8 @@ DXILBindingMap::findByUse(const Value *Key) const {
   case Intrinsic::not_intrinsic: {
     SmallVector<dxil::ResourceBindingInfo> Children;
     for (const Value *V : CI->args()) {
-      if (V->getType() != UseType) {
+      if (V->getType() != UseType)
         continue;
-      }
 
       Children.append(findByUse(V));
     }
@@ -806,8 +803,7 @@ DXILBindingMap::findByUse(const Value *Key) const {
   // Found the create, return the binding
   case Intrinsic::dx_resource_handlefrombinding:
     const auto *It = find(CI);
-    if (It == Infos.end())
-      return {};
+    assert(It != Infos.end() && "HandleFromBinding must be in resource map");
     return {*It};
   }
 

llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp

@@ -71,11 +71,8 @@ declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
     unsigned CalledResources = 0;
 
     for (const User *U : F.users()) {
-      const CallInst *CI = dyn_cast<CallInst>(U);
-      ASSERT_TRUE(CI) << "All users of @a.func must be CallInst";
-
+      const CallInst *CI = cast<CallInst>(U);
       const Value *Handle = CI->getArgOperand(0);
-
       const auto Bindings = DBM.findByUse(Handle);
       ASSERT_EQ(Bindings.size(), 1u)
           << "Handle should resolve into one resource";
@@ -124,11 +121,8 @@ declare target("dx.RawBuffer", float, 1, 0) @ind.func(target("dx.RawBuffer", flo
     unsigned CalledResources = 0;
 
     for (const User *U : F.users()) {
-      const CallInst *CI = dyn_cast<CallInst>(U);
-      ASSERT_TRUE(CI) << "All users of @a.func must be CallInst";
-
+      const CallInst *CI = cast<CallInst>(U);
       const Value *Handle = CI->getArgOperand(0);
-
       const auto Bindings = DBM.findByUse(Handle);
       ASSERT_EQ(Bindings.size(), 1u)
           << "Handle should resolve into one resource";
@@ -180,11 +174,8 @@ declare target("dx.RawBuffer", float, 1, 0) @ind.func(target("dx.RawBuffer", flo
     unsigned CalledResources = 0;
 
     for (const User *U : F.users()) {
-      const CallInst *CI = dyn_cast<CallInst>(U);
-      ASSERT_TRUE(CI) << "All users of @a.func must be CallInst";
-
+      const CallInst *CI = cast<CallInst>(U);
       const Value *Handle = CI->getArgOperand(0);
-
       const auto Bindings = DBM.findByUse(Handle);
       ASSERT_EQ(Bindings.size(), 4u)
           << "Handle should resolve into four resources";
@@ -263,11 +254,8 @@ declare target("dx.RawBuffer", float, 1, 0) @ind.func(target("dx.RawBuffer", flo
     unsigned CalledResources = 0;
 
     for (const User *U : F.users()) {
-      const CallInst *CI = dyn_cast<CallInst>(U);
-      ASSERT_TRUE(CI) << "All users of @a.func must be CallInst";
-
+      const CallInst *CI = cast<CallInst>(U);
       const Value *Handle = CI->getArgOperand(0);
-
       const auto Bindings = DBM.findByUse(Handle);
       ASSERT_EQ(Bindings.size(), 2u)
           << "Handle should resolve into four resources";