Hi,
> -----Original Message-----
> From: Segher Boessenkool [mailto:[email protected]]
> Sent: Tuesday, March 17, 2020 1:58 AM
> To: Yangfei (Felix) <[email protected]>
> Cc: [email protected]; Zhanghaijian (A) <[email protected]>
> 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
