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.
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).
> Also it's interesting to see how this may affect on those archs.
Yes. Which is why "canonicalisation" rules that are really just "this
works better on targets A and B" do not usually work well. Rules that
are predictable and that actually simplify the code might still need
all targets to update (and target maintainers will grumble, myself
included), but at least that is a way forwards (and not backwards or
sideways).
Segher
