[PATCH PR94026] combine missed opportunity to simplify comparisons with zero

[Yangfei (Felix)]

Hi,

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]
> Sent: Thursday, March 19, 2020 7:52 AM
> To: Yangfei (Felix) <felix.yang@huawei.com>
> Cc: gcc-patches@gcc.gnu.org; Zhanghaijian (A) <z.zhanghaijian@huawei.com>
> Subject: Re: [PATCH PR94026] combine missed opportunity to simplify
> comparisons with zero
> 
> Hi!
> 
> On Tue, Mar 17, 2020 at 02:05:19AM +0000, Yangfei (Felix) wrote:
> > > Trying 7 -> 8:
> > >     7: r99:SI=r103:SI>>0x8
> > >       REG_DEAD r103:SI
> > >     8: r100:SI=r99:SI&0x6
> > >       REG_DEAD r99:SI
> > > Failed to match this instruction:
> > > (set (reg:SI 100)
> > >     (and:SI (lshiftrt:SI (reg:SI 103)
> > >             (const_int 8 [0x8]))
> > >         (const_int 6 [0x6])))
> > >
> > > That should match already, perhaps with a splitter.  aarch64 does
> > > not have very generic rotate-and-mask (or -insert) instructions, so
> > > the
> > > aarch64 backend needs to help combine with the less trivial cases.
> > >
> > > If you have a splitter for *this* one, all else will probably work
> > > "automatically": you split it to two ubfm, and the second of those
> > > can then merge into the compare instruction, and everything works out.
> >
> > Do you mean splitting the above pattern into a combination of ubfx and ubfiz?
> (Both are aliases of ubfm).
> 
> Sure.  The problem with aarch's bitfield instruction is that either the source or
> the dest has to be right-aligned, which isn't natural for the compiler.
> 
> > I still don't see how the benefit can be achieved.
> > The following is the expected assembly for the test case:
> >         tst     x0, 1536
> >         cset    w0, ne
> >         ret
> > This may not happen when the remaining ubfx is there.  Also what
> instruction be matched when ubfiz is merged into the compare?
> > Anything I missed?
> 
> The second insn could combine with the compare, and then that can combine
> back further.

Let me paste the RTL input to the combine phase:
/************************
(insn 6 3 7 2 (set (reg:SI 98)
        (ashiftrt:SI (reg:SI 102)
            (const_int 8 [0x8]))) "foo.c":3:16 742 {*aarch64_ashr_sisd_or_int_si3}
     (expr_list:REG_DEAD (reg:SI 102)
        (nil)))
(note 7 6 8 2 NOTE_INSN_DELETED)
(insn 8 7 9 2 (set (reg:CC_NZ 66 cc)
        (compare:CC_NZ (and:SI (reg:SI 98)
                (const_int 6 [0x6]))
            (const_int 0 [0]))) "foo.c":5:8 698 {*andsi3nr_compare0}
     (expr_list:REG_DEAD (reg:SI 98)
        (nil)))
(note 9 8 14 2 NOTE_INSN_DELETED)
(insn 14 9 15 2 (set (reg/i:SI 0 x0)
        (ne:SI (reg:CC_NZ 66 cc)
            (const_int 0 [0]))) "foo.c":10:1 494 {aarch64_cstoresi}
     (expr_list:REG_DEAD (reg:CC 66 cc)
        (nil)))
*************************/

Two issues that I can see here:
1. When the ubfiz is combined with the compare, the combined insn does not necessarily mean a equality comparison with zero.  
  This is also the case when all the three insns (ubfx & ubfiz & compare) are combined together.  

2. Given that the patterns for ubfx and ubfiz are already not simple, I am afraid the pattern we got by combining the three would be much complex.
  And even more complex when further merged with insn 14 here in order to make sure that we are doing a equality comparison with zero.  

So it looks difficult when we go this port-specific way without matching a "zero_extact".  

> Another approach:
> 
> Trying 7 -> 9:
>     7: r99:SI=r103:SI>>0x8
>       REG_DEAD r103:SI
>     9: cc:CC_NZ=cmp(r99:SI&0x6,0)
>       REG_DEAD r99:SI
> Failed to match this instruction:
> (set (reg:CC_NZ 66 cc)
>     (compare:CC_NZ (and:SI (lshiftrt:SI (reg:SI 103)
>                 (const_int 8 [0x8]))
>             (const_int 6 [0x6]))
>         (const_int 0 [0])))
> 
> This can be recognised as just that "tst" insn, no?  But combine (or
> simplify-rtx) should get rid of the shift here, just the "and" is simpler after all (it
> just needs to change the constant for that).

No, this does not mean an equality comparison with zero.  I have mentioned this in my previous mail.  

Thanks,
Felix