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

[Yangfei (Felix)]

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]
> Sent: Friday, March 20, 2020 9:38 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
> 
> On Thu, Mar 19, 2020 at 01:43:40AM +0000, Yangfei (Felix) wrote:
> > 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.
> 
> It will be just as simple as with the other approach:

I think the problem here is how to make sure we are doing a ***equality*** comparison with zero.  
We can only do the transformation under this condition.  
Then I see combine tries the following pattern: 

173 Failed to match this instruction:
174 (set (reg:SI 101)
175     (ne:SI (and:SI (lshiftrt:SI (reg:SI 102)
176                 (const_int 8 [0x8]))
177             (const_int 6 [0x6]))
178         (const_int 0 [0])))

But this cannot match a 'tst' instruction as the above pattern does not clobber the CC flag register.  
Also this means that we depend on specific uses cases, so may need different patterns to match all possible cases.  

> > > 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.
> 
> This should be simplified to
> (set (reg:CC_NZ 66 cc)
>      (compare:CC_NZ (and:SI (reg:SI 103)
>                             (const_int 1536))
>                     (const_int 0)))
> (but it isn't), and that is just *and<mode>3nr_compare0, which is a "tst"
> instruction.  If this is fixed (in simplify-rtx.c), it will work as you want.

But I don't think it's correct for logic in simplify-rtx.c to further simplify this rtl:  

(compare:CC_NZ (and:SI (lshiftrt:SI (reg:SI 102)
            (const_int 8 [0x8]))
        (const_int 6 [0x6]))
    (const_int 0 [0]))

The reason is that it knows nothing about CC_NZ.  
CC_NZ is aarch64-port specific and it does not necessarily mean a equality comparison with zero.  
Correct me if I missed anything.  

Thanks,
Felix