[PATCH] D82703: [InstCombine] convert assumes to operand bundles

[Johannes Doerfert via Phabricator via llvm-commits]

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