Thanks for the fix and refinement!

I think the addr attr looks more reasonable, just one small issue that
EGPR was not only encoded with REX2 prefix, there are several
instructions that encode EGPR using evex prefix. So I think
addr_rex2/addr_rex may be a misleading note. I'd prefer still using
gpr16/gpr32 as the name which clearly shows which type of gpr was
adopted to an address.

Hongtao Liu <crazy...@gmail.com> 于2023年11月3日周五 20:50写道:
>
> On Fri, Nov 3, 2023 at 6:34 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > The patch generalizes address register class handling to allow multiple
> > address register classes.  For APX EGPR targets, some instructions can't be
> > encoded with REX2 prefix, so it is necessary to limit address register
> > class to avoid REX2 registers.  The same situation happens for instructions
> > with high registers, where the REX register can not be used in the address,
> > so the existing infrastructure can be adapted to also handle this case.
> >
> > The patch is mostly a mechanical rename of "gpr32" attribute to "addr" and
> > introduces no functional changes, although it fixes a couple of inconsistent
> > attribute values in passing.
>
> @@ -22569,9 +22578,8 @@ (define_insn "<sse4_1_avx2>_mpsadbw"
>     mpsadbw\t{%3, %2, %0|%0, %2, %3}
>     vmpsadbw\t{%3, %2, %1, %0|%0, %1, %2, %3}"
>    [(set_attr "isa" "noavx,noavx,avx")
> -   (set_attr "gpr32" "0,0,1")
> +   (set_attr "addr" "rex")
>     (set_attr "type" "sselog1")
> -   (set_attr "gpr32" "0")
>     (set_attr "length_immediate" "1")
>     (set_attr "prefix_extra" "1")
>     (set_attr "prefix" "orig,orig,vex")
>
> I believe your fix is correct.
>
> >
> > A follow-up patch will use the above infrastructure to limit address 
> > register
> > class to legacy registers for instructions with high registers.
>
> The patch looks good to me, but please leave some time for Hongyu in
> case he has any comments.
>
> >
> > gcc/ChangeLog:
> >
> >     * config/i386/i386.cc (ix86_memory_address_use_extended_reg_class_p):
> >     Rename to ...
> >     (ix86_memory_address_reg_class): ... this.  Generalize address
> >     register class handling to allow multiple address register classes.
> >     Return maximal class for unrecognized instructions.  Improve comments.
> >     (ix86_insn_base_reg_class): Rewrite to handle
> >     multiple address register classes.
> >     (ix86_regno_ok_for_insn_base_p): Ditto.
> >     (ix86_insn_index_reg_class): Ditto.
> >     * config/i386/i386.md: Rename "gpr32" attribute to "addr"
> >     and substitute its values with "0" -> "rex", "1" -> "*".
> >     (addr): New attribute to limit allowed address register set.
> >     (gpr32): Remove.
> >     * config/i386/mmx.md: Rename "gpr32" attribute to "addr"
> >     and substitute its values with "0" -> "rex", "1" -> "*".
> >     * config/i386/sse.md: Ditto.
> >
> > Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> >
> > Comments welcome.
> >
> > Uros.
>
>
>
> --
> BR,
> Hongtao

Reply via email to