On Fri, Sep 1, 2023 at 7:27 PM Uros Bizjak <ubiz...@gmail.com> wrote: > > On Fri, Sep 1, 2023 at 12:36 PM Hongtao Liu <crazy...@gmail.com> wrote: > > > > On Fri, Sep 1, 2023 at 5:38 PM Uros Bizjak via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > On Fri, Sep 1, 2023 at 11:10 AM Hongyu Wang <wwwhhhyyy...@gmail.com> > > > wrote: > > > > > > > > Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> 于2023年8月31日周四 > > > > 18:01写道： > > > > > > > > > > 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.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. > > > > > > > > > > > > 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. > > > > > This would be somehow similar to the SSE -> AVX512F transition, where > > > > > we still have "x" for SSE16 and "v" was introduced as a separate > > > > > register class for EVEX SSE registers. This way, asm will be > > > > > compatible, when "r", "m", "o" and "g" are used. The new memory > > > > > constraint "Bt", should allow new registers, and should be added to > > > > > the constraint string as a separate constraint, and conditionally > > > > > enabled by relevant "isa" (AKA "enabled") attribute. > > > > > > > > The extended constraint can work for registers, but for memory it is > > > > more > > > > complicated. > > > > > > Yes, unfortunately. The compiler assumes that an unchangeable register > > > class is used for BASE/INDEX registers. I have hit this limitation > > > when trying to implement memory support for instructions involving > > > 8-bit high registers (%ah, %bh, %ch, %dh), which do not support REX > > > registers, also inside memory operand. (You can see the "hack" in e.g. > > > *extzvqi_mem_rex64" and corresponding peephole2 with the original > > > *extzvqi pattern). I am aware that dynamic insn-dependent BASE/INDEX > > > register class is the major limitation in the compiler, so perhaps the > > > strategy on how to override this limitation should be discussed with > > > the register allocator author first. Perhaps adding an insn attribute > > > to insn RTX pattern to specify different BASE/INDEX register sets can > > > be a better solution than passing insn RTX to the register allocator. > > > > > > The above idea still does not solve the asm problem on how to select > > > correct BASE/INDEX register set for memory operands. > > The current approach disables gpr32 for memory operand in asm_operand > > by default. but can be turned on by options > > ix86_apx_inline_asm_use_gpr32(users need to guarantee the instruction > > supports gpr32). > > Only ~ 5% of total instructions don't support gpr32, reversed approach > > only gonna get more complicated. > > I'm not referring to the reversed approach, just want to point out > that the same approach as you proposed w.r.t. to memory operand can be > achieved using some named insn attribute that would affect BASE/INDEX > register class selection. The attribute could default to gpr32 with > APX, unless the insn specific attribute has e.g. nogpr32 value. See > for example how "enabled" and "preferred_for_*" attributes are used. > Perhaps this new attribute can also be applied to separate > alternatives. Yes, for xop/fma4/3dnow instructions, I think we can use isa attr like (define_attr "gpr32" "0, 1" (cond [(eq_attr "isa" "fma4") (const_string "0")] (const_string "1")))
But still, we need to adjust memory constraints in the pattern. Ideally, gcc includes encoding information for every instruction, (.i.e. map0/map1), so that we can determine the attribute value of gpr32 directly from this information. > > Uros. -- BR, Hongtao
