On Fri, Nov 3, 2023 at 2:21 PM Hongyu Wang <wwwhhhyyy...@gmail.com> wrote:
>
> 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.
No problem, I will keep gpr8/gpr16/gpr32 as "addr" values.
Thanks,
Uros.
>
> 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
