[PATCH] D148210: [InstCombine] icmp(MulC * X * Y, C) --> icmp(X * Y, C)

[Jun Zhang via Phabricator via llvm-commits]

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

In D148210#4271835 <https://reviews.llvm.org/D148210#4271835>, @nikic wrote:

> In D148210#4271709 <https://reviews.llvm.org/D148210#4271709>, @junaire wrote:
>
>> I tried to unify `icmp((MulC * X) * Y, C)` and `icmp(X * MulC, C)` but it looks like adds more complexity. Because we have to extract the `MulC` bit by bit in case 1 in order to also get the `nsw` flags. And for case 2 `MulC` is needed in the below folds (MulC can't be zero or `X * MulC == C --> X == C/MulC` doesn't work anymore). What's more `C < 0` is OK for case 2 but it doesn't apply to case 1. That said, though these cases have almost the same effect, they don't actually share too much common code...
>
> I'm not sure what you mean here. Why does MulC need to be extracted bit by bit? Don't we just need to implement these two rules on whether nuw/nsw can be preserved during reassociation? https://alive2.llvm.org/ce/z/KKtP62 https://alive2.llvm.org/ce/z/jgrYGB

Thanks, I think I get your ideas. Note I didn't add folds for `nuw` since I found the existing logic works great for it (https://alive2.llvm.org/ce/z/r_oHlt)
Note we should also reassociate for `shl` since `X * Power_Of_N` will be fold to `X << N`, I left it as a TODO. But I'm OK with that if you want to do it in this patch :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148210