[PATCH] D136015: [InstCombine] Fold series of instructions into mull

[Paweł Bylica via Phabricator via llvm-commits]

chfast added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/mul_full_64.ll:452
 
 define i64 @mullo(i64 %x, i64 %y) {
 ; CHECK-LABEL: @mullo(
----------------
Allen wrote:
> spatel wrote:
> > chfast wrote:
> > > Interestingly, it hasn't folded this one.
> > This patch assumes we are ending with an "add", but this test changes to an "or". We'd need to add another check for hasNoCommonBitsSet() to catch it?
> > 
> > Here's another potential fold:
> > https://alive2.llvm.org/ce/z/hUm56R
> > ...but it needs to freeze the inputs to be poison-safe because they have multiple uses.
> hi @chfast
>    I think the case **@mullo** should not be matched? https://alive2.llvm.org/ce/z/jH4kU7
> 
> hi, @spatel
>    As the case in link https://alive2.llvm.org/ce/z/hUm56R, it's result not equal to **mul i8 %y, %x**, so it need some other logic to match ? maybe defined with a new helper function. see detail https://alive2.llvm.org/ce/z/FEgEU7
>   ```
> define i8 @tgt(i8 %x, i8 %y) {
>   %m = mul i8 %y, %x
>   ret i8 %m
> }
>   ```
>   I think the case **@mullo** should not be matched? https://alive2.llvm.org/ce/z/jH4kU7

There is a typo in the example. You changed `or` to `and` but the original pattern starts at `add`. I.e. all patterns starting at `add`, `or` and `xor` should work, the one starting at `and` should not. https://alive2.llvm.org/ce/z/y26zaW

I'm not sure it is worth to expand the matching to `or` and `xor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136015