On Thu, Aug 31, 2023 at 10:20 AM Hongyu Wang <hongyu.w...@intel.com> wrote:
>
> From: Kong Lingling <lingling.k...@intel.com>
>
> These legacy insn in opcode map0/1 only support GPR16,
> and do not have vex/evex counterpart, directly adjust constraints and
> add gpr32 attr to patterns.
>
> insn list:
> 1. xsave/xsave64, xrstor/xrstor64
> 2. xsaves/xsaves64, xrstors/xrstors64
> 3. xsavec/xsavec64
> 4. xsaveopt/xsaveopt64
> 5. fxsave64/fxrstor64

IMO, instructions should be handled with a reversed approach. Add "h"
constraint (and memory constraint that can handle EGPR) to
instructions that CAN use EGPR (together with a relevant "enabled"
attribute. We have had the same approach with "x" to "v" transition
with SSE registers. If we "forgot" to add "v" to the instruction, it
still worked, but not to its full potential w.r.t available registers.

Uros.
>
> gcc/ChangeLog:
>
>         * config/i386/i386.md (<xsave>): Set attr gpr32 0 and constraint
>         Bt.
>         (<xsave>_rex64): Likewise.
>         (<xrstor>_rex64): Likewise.
>         (<xrstor>64): Likewise.
>         (fxsave64): Likewise.
>         (fxstore64): Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * lib/target-supports.exp: Add apxf check.
>         * gcc.target/i386/apx-legacy-insn-check-norex2.c: New test.
>         * gcc.target/i386/apx-legacy-insn-check-norex2-asm.c: New assembler 
> test.
> ---
>  gcc/config/i386/i386.md                       | 18 +++++++----
>  .../i386/apx-legacy-insn-check-norex2-asm.c   |  5 ++++
>  .../i386/apx-legacy-insn-check-norex2.c       | 30 +++++++++++++++++++
>  gcc/testsuite/lib/target-supports.exp         | 10 +++++++
>  4 files changed, 57 insertions(+), 6 deletions(-)
>  create mode 100644 
> gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2-asm.c
>  create mode 100644 
> gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2.c
>
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index b9eaea78f00..83ad01b43c1 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -25626,11 +25626,12 @@ (define_insn "fxsave"
>          (symbol_ref "ix86_attr_length_address_default (insn) + 3"))])
>
>  (define_insn "fxsave64"
> -  [(set (match_operand:BLK 0 "memory_operand" "=m")
> +  [(set (match_operand:BLK 0 "memory_operand" "=Bt")
>         (unspec_volatile:BLK [(const_int 0)] UNSPECV_FXSAVE64))]
>    "TARGET_64BIT && TARGET_FXSR"
>    "fxsave64\t%0"
>    [(set_attr "type" "other")
> +   (set_attr "gpr32" "0")
>     (set_attr "memory" "store")
>     (set (attr "length")
>          (symbol_ref "ix86_attr_length_address_default (insn) + 4"))])
> @@ -25646,11 +25647,12 @@ (define_insn "fxrstor"
>          (symbol_ref "ix86_attr_length_address_default (insn) + 3"))])
>
>  (define_insn "fxrstor64"
> -  [(unspec_volatile [(match_operand:BLK 0 "memory_operand" "m")]
> +  [(unspec_volatile [(match_operand:BLK 0 "memory_operand" "Bt")]
>                     UNSPECV_FXRSTOR64)]
>    "TARGET_64BIT && TARGET_FXSR"
>    "fxrstor64\t%0"
>    [(set_attr "type" "other")
> +   (set_attr "gpr32" "0")
>     (set_attr "memory" "load")
>     (set (attr "length")
>          (symbol_ref "ix86_attr_length_address_default (insn) + 4"))])
> @@ -25704,7 +25706,7 @@ (define_insn "<xsave>"
>          (symbol_ref "ix86_attr_length_address_default (insn) + 3"))])
>
>  (define_insn "<xsave>_rex64"
> -  [(set (match_operand:BLK 0 "memory_operand" "=m")
> +  [(set (match_operand:BLK 0 "memory_operand" "=Bt")
>         (unspec_volatile:BLK
>          [(match_operand:SI 1 "register_operand" "a")
>           (match_operand:SI 2 "register_operand" "d")]
> @@ -25713,11 +25715,12 @@ (define_insn "<xsave>_rex64"
>    "<xsave>\t%0"
>    [(set_attr "type" "other")
>     (set_attr "memory" "store")
> +   (set_attr "gpr32" "0")
>     (set (attr "length")
>          (symbol_ref "ix86_attr_length_address_default (insn) + 3"))])
>
>  (define_insn "<xsave>"
> -  [(set (match_operand:BLK 0 "memory_operand" "=m")
> +  [(set (match_operand:BLK 0 "memory_operand" "=Bt")
>         (unspec_volatile:BLK
>          [(match_operand:SI 1 "register_operand" "a")
>           (match_operand:SI 2 "register_operand" "d")]
> @@ -25726,6 +25729,7 @@ (define_insn "<xsave>"
>    "<xsave>\t%0"
>    [(set_attr "type" "other")
>     (set_attr "memory" "store")
> +   (set_attr "gpr32" "0")
>     (set (attr "length")
>          (symbol_ref "ix86_attr_length_address_default (insn) + 4"))])
>
> @@ -25743,7 +25747,7 @@ (define_insn "<xrstor>"
>
>  (define_insn "<xrstor>_rex64"
>     [(unspec_volatile:BLK
> -     [(match_operand:BLK 0 "memory_operand" "m")
> +     [(match_operand:BLK 0 "memory_operand" "Bt")
>        (match_operand:SI 1 "register_operand" "a")
>        (match_operand:SI 2 "register_operand" "d")]
>       ANY_XRSTOR)]
> @@ -25751,12 +25755,13 @@ (define_insn "<xrstor>_rex64"
>    "<xrstor>\t%0"
>    [(set_attr "type" "other")
>     (set_attr "memory" "load")
> +   (set_attr "gpr32" "0")
>     (set (attr "length")
>          (symbol_ref "ix86_attr_length_address_default (insn) + 3"))])
>
>  (define_insn "<xrstor>64"
>     [(unspec_volatile:BLK
> -     [(match_operand:BLK 0 "memory_operand" "m")
> +     [(match_operand:BLK 0 "memory_operand" "Bt")
>        (match_operand:SI 1 "register_operand" "a")
>        (match_operand:SI 2 "register_operand" "d")]
>       ANY_XRSTOR64)]
> @@ -25764,6 +25769,7 @@ (define_insn "<xrstor>64"
>    "<xrstor>64\t%0"
>    [(set_attr "type" "other")
>     (set_attr "memory" "load")
> +   (set_attr "gpr32" "0")
>     (set (attr "length")
>          (symbol_ref "ix86_attr_length_address_default (insn) + 4"))])
>
> diff --git a/gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2-asm.c 
> b/gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2-asm.c
> new file mode 100644
> index 00000000000..7ecc861435f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2-asm.c
> @@ -0,0 +1,5 @@
> +/* { dg-do assemble { target apxf } } */
> +/* { dg-options "-O1 -mapxf -m64 -DDTYPE32" } */
> +
> +#include "apx-legacy-insn-check-norex2.c"
> +
> diff --git a/gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2.c 
> b/gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2.c
> new file mode 100644
> index 00000000000..1e5450dfb73
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/apx-legacy-insn-check-norex2.c
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -mapxf -m64 -DDTYPE32" } */
> +
> +#include <immintrin.h>
> +
> +typedef unsigned int u32;
> +typedef unsigned long long u64;
> +
> +#ifndef DTYPE32
> +#define DTYPE32
> +#endif
> +
> +#ifdef DTYPE32
> +typedef u32 DTYPE;
> +#endif
> +
> +__attribute__((target("xsave,fxsr")))
> +void legacy_test ()
> +{
> +  register DTYPE* val __asm__("r16");
> +  _xsave64 (val, 1);
> +  _xrstor64 (val, 1);
> +  _fxsave64 (val);
> +  _fxrstor64 (val);
> +}
> +
> +/* { dg-final { scan-assembler-not "xsave64\[ 
> \\t]+\\\.\\\*r\(1\[6-9\]\|2\[0-9\]|30\|31\)" } } */
> +/* { dg-final { scan-assembler-not "xrstor64\[ 
> \\t]+\\\.\\\*r\(1\[6-9\]\|2\[0-9\]|30\|31\)" } } */
> +/* { dg-final { scan-assembler-not "fxsave64\[ 
> \\t]+\\\.\\\*r\(1\[6-9\]\|2\[0-9\]|30\|31\)" } } */
> +/* { dg-final { scan-assembler-not "fxrstor64\[ 
> \\t]+\\\.\\\*r\(1\[6-9\]\|2\[0-9\]|30\|31\)" } } */
> diff --git a/gcc/testsuite/lib/target-supports.exp 
> b/gcc/testsuite/lib/target-supports.exp
> index d353cc0aaf0..6359408542a 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -9938,6 +9938,16 @@ proc check_effective_target_sm4 { } {
>      } "-msm4" ]
>  }
>
> +proc check_effective_target_apxf { } {
> +    return [check_no_compiler_messages apxf object {
> +       void
> +       foo ()
> +       {
> +         __asm__ volatile ("add\t%%r16, %%r31" ::);
> +       }
> +    } "-mapxf" ]
> +}
> +
>  # Return 1 if sse instructions can be compiled.
>  proc check_effective_target_sse { } {
>      return [check_no_compiler_messages sse object {
> --
> 2.31.1
>

Reply via email to