[PATCH] D140850: [Patch 2/4]: Add optimizations for icmp eq/ne (mul(X, Y), 0)

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

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

> In D140850#4025580 <https://reviews.llvm.org/D140850#4025580>, @nikic wrote:
>
>> Proofs:
>>
>> Both odd: https://alive2.llvm.org/ce/z/9qgwMo
>> One odd: https://alive2.llvm.org/ce/z/vRqUQO
>> Non-zero nuw: https://alive2.llvm.org/ce/z/3Bqx2-
>> Non-zero nsw: https://alive2.llvm.org/ce/z/AybG6r
>
> Thanks for writing those.
>
> For future reference, I ran `alive2` on the entire test file locally (basically `opt -passes=instcombine tests-before.ll -o tests-after.ll; ./alive2 tests-before.ll tests-after.ll` Is there a way to get that into godbolt or is the only way to do 1 function at a time with `src` and `tgt`?

I think it only works with src/tgt. It is also possible to let alive-tv run passes over the input, but of course that requires the changes to already be implemented.

>> From the test diffs, I don't see any cases where we actually hit the true/false case. Presumably, this get's reliably handled by the "icmp eq isKnownNonZero, 0" fold in InstSimplify. If that's the case, we can omit handling for that.
>
> We start hitting it in: https://reviews.llvm.org/D140851
> See: `define i64 @mul_assume_V_oddV_s64_setz(i64 %v, i64 %other)`
>
> Before then the missing cases in analysis was getting in the way. Could probably create a case where
> it works at this patch. Would you like to add one?

It's not really clear to me that that folds only in conjunction with this patch. I suspect that one also folds via InstSimplify, just with strengthened assume handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140850