[PATCH] D77402: [AssumeBundles] adapt Assumption cache to assume bundles

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

One more conceptual question and small nits.



================
Comment at: llvm/lib/Analysis/AssumptionCache.cpp:60
+findAffectedValues(CallInst *CI,
+                   SmallVectorImpl<std::pair<Value *, unsigned>> &Affected) {
   // Note: This code must be kept in-sync with the code in
----------------
I usually prefer a struct instead of a pair but this is local only so I guess it's ok. Feel free to use the `ResultElem` or something similar to improve readability here and at the use site below. (`AV.first` will then become something meaningful on first glance.)


================
Comment at: llvm/lib/Analysis/AssumptionCache.cpp:155
+      if (!HasNonnull)
+        AffectedValues.erase(AVI);
+    }
----------------
Could you explain the need for the above logic please? Is is strictly related to the index extension or a general improvement?


(Nit: Use a continue to reduce indention and simplify the code)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77402/new/

https://reviews.llvm.org/D77402