[PATCH] D140849: [Patch 1/4]: Add tests for binops with conditions/assume constraints

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-binop.ll:15
+  %tobool.not = icmp eq i64 %mul, 0
+  %conv = zext i1 %tobool.not to i64
+  ret i64 %conv
----------------
nikic wrote:
> All these zext + ret and be replaced by `ret i1`.
> All these zext + ret and be replaced by `ret i1`.

Figured no difference. Do you want all unnecessary zext removed?


================
Comment at: llvm/test/Transforms/InstCombine/icmp-binop.ll:295
+  tail call void @llvm.assume(i1 %or.cond7)
+  ret i64 0
+}
----------------
nikic wrote:
> What's the point of these these tests that contain only assume calls?
These tests are more for patterns 3/4.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-binop.ll:615
+  %tobool.not = icmp eq i64 %v, 0
+  %conv = zext i1 %tobool.not to i64
+  ret i64 %conv
----------------
nikic wrote:
> What's the point of these tests?
> What's the point of these tests?

None, I write the tests in C -> ll. Guess this one got optimized. I'll drop in next revision to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140849