[PATCH] D77402: [AssumeBundles] adapt Assumption cache to assume bundles
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 9 17:58:43 PDT 2020
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
More information about the llvm-commits
mailing list