[PATCH] D82703: [InstCombine] convert assumes to operand bundles
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 31 10:57:40 PST 2021
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.
One minor comment and a nit, I assume those can be addressed before committing the patch. LGTM, thanks for finishing this up!
================
Comment at: llvm/lib/Analysis/Loads.cpp:102
+ return true;
+ }
+ /// TODO refactor this function to be able to search independently for
----------------
Nit: If you move the lambda declaration before the conditional, we get one level of indirection less.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1619
+ II, RK, &getAssumptionCache(), &getDominatorTree());
+ if (CanonRK != RK) {
+ if (!CanonRK) {
----------------
Nit: early continue, `if (CanonRK == RK) continue`
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1635
+ Worklist.pushValue(RK.WasOn);
+ return II;
+ }
----------------
Again, unsure why we return. there might be operand bundles left.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82703/new/
https://reviews.llvm.org/D82703
More information about the llvm-commits
mailing list