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

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM

In D136015#3879280 <https://reviews.llvm.org/D136015#3879280>, @Allen wrote:

> Done, thanks very much for your changes. And I don't completely understand why need the use at the beginning of a function? eg:
>
>   define i8 @mul8_low_A0_B2(i8 %in0, i8 %p) {
>     %in1 = call i8 @use8(i8 %p) ; thwart complexity-based canonicalization

If you remove that line, notice that the values in the later multiply get commuted. That happens before we reach this transform, so the test is trying to ensure that the exact placement of the values at runtime is the same as specified in the test.



================
Comment at: llvm/test/Transforms/InstCombine/mul_full_64.ll:452
 
 define i64 @mullo(i64 %x, i64 %y) {
 ; CHECK-LABEL: @mullo(
----------------
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.


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

https://reviews.llvm.org/D136015