Re: [PATCH 06/13] [APX EGPR] Map reg/mem constraints in inline asm to non-EGPR constraint.

[Hongtao Liu <crazylht@gmail.com>]

On Fri, Sep 1, 2023 at 7:03 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On Thu, Aug 31, 2023 at 11:18 AM Jakub Jelinek via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> On Thu, Aug 31, 2023 at 04:20:17PM +0800, Hongyu Wang via Gcc-patches wrote:
> >> > From: Kong Lingling <lingling.kong@intel.com>
> >> >
> >> > In inline asm, we do not know if the insn can use EGPR, so disable EGPR
> >> > usage by default from mapping the common reg/mem constraint to non-EGPR
> >> > constraints. Use a flag mapx-inline-asm-use-gpr32 to enable EGPR usage
> >> > for inline asm.
> >> >
> >> > gcc/ChangeLog:
> >> >
> >> >       * config/i386/i386.cc (INCLUDE_STRING): Add include for
> >> >       ix86_md_asm_adjust.
> >> >       (ix86_md_asm_adjust): When APX EGPR enabled without specifying the
> >> >       target option, map reg/mem constraints to non-EGPR constraints.
> >> >       * config/i386/i386.opt: Add option mapx-inline-asm-use-gpr32.
> >> >
> >> > gcc/testsuite/ChangeLog:
> >> >
> >> >       * gcc.target/i386/apx-inline-gpr-norex2.c: New test.
> >> > ---
> >> >  gcc/config/i386/i386.cc                       |  44 +++++++
> >> >  gcc/config/i386/i386.opt                      |   5 +
> >> >  .../gcc.target/i386/apx-inline-gpr-norex2.c   | 107 ++++++++++++++++++
> >> >  3 files changed, 156 insertions(+)
> >> >  create mode 100644 gcc/testsuite/gcc.target/i386/apx-inline-gpr-norex2.c
> >> >
> >> > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> >> > index d26d9ab0d9d..9460ebbfda4 100644
> >> > --- a/gcc/config/i386/i386.cc
> >> > +++ b/gcc/config/i386/i386.cc
> >> > @@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public License
> >> >  along with GCC; see the file COPYING3.  If not see
> >> >  <http://www.gnu.org/licenses/>.  */
> >> >
> >> > +#define INCLUDE_STRING
> >> >  #define IN_TARGET_CODE 1
> >> >
> >> >  #include "config.h"
> >> > @@ -23077,6 +23078,49 @@ ix86_md_asm_adjust (vec<rtx> &outputs, vec<rtx> & /*inputs*/,
> >> >    bool saw_asm_flag = false;
> >> >
> >> >    start_sequence ();
> >> > +  /* TODO: Here we just mapped the general r/m constraints to non-EGPR
> >> > +   constraints, will eventually map all the usable constraints in the future. */
> >>
> >> I think there should be some constraint which explicitly has all the 32
> >> GPRs, like there is one for just all 16 GPRs (h), so that regardless of
> >> -mapx-inline-asm-use-gpr32 one can be explicit what the inline asm wants.
> >>
> >> Also, what about the "g" constraint?  Shouldn't there be another for "g"
> >> without r16..r31?  What about the various other memory
> >> constraints ("<", "o", ...)?
> >
> > I think we should leave all existing constraints as they are, so "r"
> > covers only GPR16, "m" and "o" to only use GPR16. We can then
> > introduce "h" to instructions that have the ability to handle EGPR.
>
> Yeah.  I'm jumping in without having read the full thread, sorry,
> but the current mechanism for handling this is TARGET_MEM_CONSTRAINT
> (added for s390).  That is, TARGET_MEM_CONSTRAINT can be defined to some
Thanks for the comments.
> new constraint that is more general than the traditional "m" constraint.
> This constraint is then the one that is associated with memory_operand
> etc.  "m" can then be defined explicitly to the old definition,
> so that existing asms continue to work.
>
> So if the port wants generic internal memory addresses to use the
> EGPR set (sounds reasonable), then TARGET_MEM_CONSTRAINT would be
> a new constraint that maps to those addresses.
But still we need to enhance current reload infrastructure to support
selective base_reg_class/index_reg_class, refer to [1].
The good thing about using TARGET_MEM_CONSTRAINT is that we don't have
to remapping memory constraint for inline asm, but the bad thing about
it is that we need to modify the backend pattern a lot, because only
5% of the instructions don't support gpr32, and 95% of them need to be
changed to the new memory constraint.
It feels like the cons outweigh the pros.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-September/629040.html

>
> Thanks,
> Richard



-- 
BR,
Hongtao