Hi,

> -----Original Message-----
> From: Segher Boessenkool [mailto:seg...@kernel.crashing.org]
> Sent: Tuesday, March 17, 2020 1:58 AM
> To: Yangfei (Felix) <felix.y...@huawei.com>
> Cc: gcc-patches@gcc.gnu.org; Zhanghaijian (A) <z.zhanghaij...@huawei.com>
> Subject: Re: [PATCH PR94026] combine missed opportunity to simplify
> comparisons with zero
> 
> On Mon, Mar 16, 2020 at 06:29:39AM +0000, Yangfei (Felix) wrote:
> > Sorry for not getting your point here.
> 
> 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).  
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?  

> > Also, this issue is there for ports like x86.  If we go that way, then we 
> > need
> to handle each port affected.
> 
> Yes, you need to do target-specific stuff in every backend separately.
> 
> > So I am inclined to handle this in an arch-independent way.
> 
> But you don't.  The transformation you do looks to be actively harmful on
> many targets.  (I haven't tested it yet, that takes 8h currently).

Now I know your concern about zero_extract.  Maybe this should be mentioned in 
docs like gccint.  
Also it's interesting to see how this may affect on those archs.  

Thanks,
Felix

Reply via email to