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

[Allen zhong via Phabricator via llvm-commits]

Allen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1296
+      !match(YLo, m_And(m_Value(Y), m_SpecificInt(HalfMask))))
+    return nullptr;
+
----------------
RKSimon wrote:
> Allen wrote:
> > RKSimon wrote:
> > > What about if the AND has been removed by SimplifyDemandedBits? Maybe also test for KnownBits known leading zeros?
> > Thanks for your suggestion, I'll record this issue, and I'll try out your suggestions later with a separate patch ?
> A TODO comment is fine for now - cheers
hi @RKSimon 
  As this revision is accept, so it is time to consider your refactor suggestion, do you have some idea about the extra tests ? thanks.


================
Comment at: llvm/test/Transforms/InstCombine/mul_full_64.ll:452
 
 define i64 @mullo(i64 %x, i64 %y) {
 ; CHECK-LABEL: @mullo(
----------------
chfast wrote:
> 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.
Thanks @chfast for your case. I take a look at your case more, except the above **add **VS **or**, there is some other diffirence with my initail case. https://alive2.llvm.org/ce/z/ZKmrJB


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136015