Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> 于2023年8月31日周四 17:18写道：
>
> On Thu, Aug 31, 2023 at 04:20:17PM +0800, Hongyu Wang via Gcc-patches wrote:
> > From: Kong Lingling <lingling.k...@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.
>
Yes, we will add new register constraints. For memory constraints it requires
some special handling in ix86_memory_address_use_extended_reg_class_p
> Also, what about the "g" constraint? Shouldn't there be another for "g"
> without r16..r31? What about the various other memory
> constraints ("<", "o", ...)?
We will support fully mapping of all common constraints, with refining
of current
mapping in the V2 patch.
>
> > + if (TARGET_APX_EGPR && !ix86_apx_inline_asm_use_gpr32)
> > + {
> > + /* Map "r" constraint in inline asm to "h" that disallows r16-r31
> > + and replace only r, exclude Br and Yr. */
> > + for (unsigned i = 0; i < constraints.length (); i++)
> > + {
> > + std::string *s = new std::string (constraints[i]);
>
> Doesn't this leak memory (all the time)?
> I must say I don't really understand why you need to use std::string here,
> but certainly it shouldn't leak.
std::string just makes the code shorter than using str functions. Current code
will be completely refactored with supporting more mapping of constraints.
>
> > + size_t pos = s->find ('r');
> > + while (pos != std::string::npos)
> > + {
> > + if (pos > 0
> > + && (s->at (pos - 1) == 'Y' || s->at (pos - 1) == 'B'))
> > + pos = s->find ('r', pos + 1);
> > + else
> > + {
> > + s->replace (pos, 1, "h");
> > + constraints[i] = (const char*) s->c_str ();
>
> Formatting (space before *). The usual way for constraints is ggc_strdup on
> some string in a buffer. Also, one could have several copies or r (or m,
> memory (doesn't
> that appear just in clobbers? And that doesn't look like something that
> should be replaced), Bm, e.g. in various alternatives. So, you
> need to change them all, not just the first hit. "r,r,r,m" and the like.
> Normally, one would simply walk the constraint string, parsing the special
> letters (+, =, & etc.) and single letter constraints and 2 letter
> constraints using CONSTRAINT_LEN macro (tons of examples in GCC sources).
> Either do it in 2 passes, first one counts how long constraint string one
> will need after the adjustments (and whether to adjust something at all),
> then if needed XALLOCAVEC it and adjust in there, or say use a
> auto_vec<char, 32> for
> it.
Thanks for your guidance. Previously we thought constraints[i] was a splitted
simple constraint, but clearly it is not.
We will refer to an existing example of this and rewrite current one.
>
> > + break;
> > + }
> > + }
> > + }
> > + /* Also map "m/memory/Bm" constraint that may use GPR32, replace
> > them with
> > + "Bt/Bt/BT". */
> > + for (unsigned i = 0; i < constraints.length (); i++)
> > + {
> > + std::string *s = new std::string (constraints[i]);
> > + size_t pos = s->find ("m");
> > + size_t pos2 = s->find ("memory");
> > + if (pos != std::string::npos)
> > + {
> > + if (pos > 0 && (s->at (pos - 1) == 'B'))
> > + s->replace (pos - 1, 2, "BT");
> > + else if (pos2 != std::string::npos)
> > + s->replace (pos, 6, "Bt");
> > + else
> > + s->replace (pos, 1, "Bt");
>
> Formatting, the s->replace calls are indented too much.
>
> Jakub
>
