[PATCH] D151694: [InstCombine] Add test case for (icmp eq A, -1) & (icmp eq B, -1) --> (icmp eq (A&B), -1); NFC

Tue May 30 21:07:37 PDT 2023

xgupta marked an inline comment as done.
xgupta added a comment.

In D151694#4381454 <https://reviews.llvm.org/D151694#4381454>, @goldstein.w.n wrote:

> You have the parent-child relationship of the test/impl patches backwards. This should be the parent of D151660 <https://reviews.llvm.org/D151660>, not the otherway around.

Thanks, updated now, It was a confusion.

================
Comment at: llvm/test/Transforms/InstCombine/pr62311.ll:297
+lor.end:                                          ; preds = %lor.rhs, %entry
+  %0 = phi i1 [ true, %entry ], [ %cmp19, %lor.rhs ]
+  ret i1 %0
----------------
goldstein.w.n wrote:
> Personally, think all of these test cases are more complex than what we need.
> 
> Something simple like:
> ```
> define i1 @icmp_eq_m1_and_eq_m1(i8 %x, i8 %y) {
>   %rx = icmp eq i8 %x, -1
>   %ry = icmp eq i8 %y, -1
>   %r = and i1 %rx, %ry
>   ret i1 %r
> }
> ```
> 
> Is probably more inline with what we want. One more thing. Can you please postfix "_fail" to all the negative tests so it clear to the reader.
> 
> Also can you rename the file to give it a more descriptive name. Maybe something like:
> `and-or-icmp-equality.ll` or you could just put these tests in `and-or-icmps.ll`
Thanks, Updated the test cases and put them in and-or-icmps.ll

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151694