[PATCH] D77804: [DAG] Enable ISD::SRL SimplifyMultipleUseDemandedBits handling inside SimplifyDemandedBits (WIP)

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/test/CodeGen/ARM/uxtb.ll:112
+; CHECK-NEXT:    orr r0, r0, r1
 ; CHECK-NEXT:    bx lr
   %tmp1 = lshr i32 %p0, 7
----------------
RKSimon wrote:
> dmgreen wrote:
> > RKSimon wrote:
> > > RKSimon wrote:
> > > > I'm going to take a look at this, but I'm really not familiar with the UXTB matching code, so any pointers would be appreciated.
> > > instcombine optimises this as well:
> > > ```
> > > define i32 @test10(i32 %p0) {
> > >   %tmp1 = lshr i32 %p0, 7
> > >   %tmp2 = and i32 %tmp1, 16253176
> > >   %tmp4 = lshr i32 %p0, 12
> > >   %tmp5 = and i32 %tmp4, 458759
> > >   %tmp7 = or i32 %tmp5, %tmp2
> > >   ret i32 %tmp7
> > > }
> > > ```
> > > which has the same problem:
> > > ```
> > > _test10:
> > > @ %bb.0:
> > >         mov     r1, #248
> > >         mov     r2, #7
> > >         orr     r1, r1, #16252928
> > >         orr     r2, r2, #458752
> > >         and     r1, r1, r0, lsr #7
> > >         and     r0, r2, r0, lsr #12
> > >         orr     r0, r0, r1
> > >         bx      lr
> > > ```
> > I was taking a look. The test is super old now, so old that it had signed types when it was originally added.
> > 
> > I was surprised to see that `and 0x70007` is being recognised via an `and 0xff00ff` tablegen pattern - it goes into SelectionDAGISel::CheckAndMask which checks that the other mask bits are already 0.
> > 
> > I think that is what this is trying to test - that a smaller and mask still matches the UXTB16. Is it possible to change it to something that still captures that, without relying on the multi-use fold of the %tmp2 not happening?
> > 
> > Maybe something like this?
> > ```
> >   %p = and i32 %p0, 3
> >   %a = shl i32 65537, %p
> >   %b = lshr i32 %a, 1
> >   %tmp7 = and i32 %b, 458759
> > ```
> Thanks for the hint - I'll give it a try
Thanks @dmgreen - those still match fine. Should I pre-commit these new tests and possibly alter the existing test10 variants with the -instcombine optimized IR to show they already fail to match?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77804