Re: [PATCH] Intrinsics for fxsave[,64], xsave[,64], xsaveopt[,64]

[Uros Bizjak <ubizjak at gmail dot com>]

On Thu, Oct 18, 2012 at 11:16 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
> Hi,
>
> this patch adds new intrinsics for fxsave, fxsave64, xsave, xsave64,
> xsaveopt and xsaveopt64 instructions
>
> Bootstrapped on x86-64
>
> Is it ok for trunk?
>
> Changelog entry:
> 2012-10-16  Alexander Ivchenko  <alexander.ivchenko@intel.com>
>
>         * gcc/common/config/i386/i386-common.c
>         (OPTION_MASK_ISA_FXSAVE_SET): New.
>         (OPTION_MASK_ISA_XSAVE_SET): Likewise.
>         (ix86_handle_option): Handle mfxsave and mxsave options.
>         * gcc/config.gcc (i[34567]86-*-*): Add fxsaveintrin.h and xsaveintrin.h.
>         (x86_64-*-*): Likewise.
>         * config/i386/xsaveintrin.h: New header.
>         * config/i386/fxsaveintrin.h: Likewise.
>         * gcc/config/i386/driver-i386.c (host_detect_local_cpu): Detect
>         FXSAVE/XSAVE support.
>         * gcc/config/i386/i386-builtin-types.def
>         (VOID_FTYPE_PVOID_INT64): New function type.
>         * gcc/config/i386/i386-c.c: Define __FXSAVE__ and __XSAVE__ if needed.
>         * gcc/config/i386/i386.c (ix86_target_string): Define -mfxsave
>         and -mxsave options.
>         (PTA_FXSAVE): New.
>         (PTA_XSAVE): Likewise.
>         (ix86_option_override_internal): Handle new option.
>         (ix86_valid_target_attribute_inner_p): Add OPT_mfxsave, OPT_mxsave.
>         (ix86_builtins): Add IX86_BUILTIN_FXSAVE32, IX86_BUILTIN_FXSAVE64,
>         IX86_BUILTIN_XSAVE32, IX86_BUILTIN_XSAVE64,
>         IX86_BUILTIN_XSAVEOPT32, IX86_BUILTIN_XSAVEOPT64.
>         (ix86_expand_builtin): Handle these built-ins.
>         * gcc/config/i386/i386.h (TARGET_FXSAVE): New.
>         (TARGET_XSAVE): Likewise.
>         * gcc/config/i386/i386.md (fxsave32): New.
>         (fxsave64): Likewise.
>         (xsave32): Likewise.
>         (xsave64): Likewise.
>         (xsaveopt32): Likewise.
>         (xsaveopt64): Likewise.
>         * gcc/config/i386/i386.opt (mfxsave): New.
>         (mxsave): Likewise.
>         * gcc/config/i386/x86intrin.h: Include
>         xsaveintrin.h, fxsaveintrin.h.
>
> testsuite/Changelog entry:
> 2012-10-16  Alexander Ivchenko  <alexander.ivchenko@intel.com>
>
>         * gcc.target/i386/fxsave-1.c: New.
>         * gcc.target/i386/fxsave64-1.c: Ditto.
>         * gcc.target/i386/xsave-1.c: Ditto.
>         * gcc.target/i386/xsave64-1.c: Ditto.
>         * gcc.target/i386/xsaveopt-1.c: Ditto.
>         * gcc.target/i386/xsaveopt64-1.c: Ditto.

+    case IX86_BUILTIN_FXSAVE32:
+      arg0 = CALL_EXPR_ARG (exp, 0);
+      op0 = expand_normal (arg0);
+
+      if (GET_MODE (op0) != Pmode)
+	op0 = convert_to_mode (Pmode, op0, 1);
+      op0 = gen_rtx_MEM (Pmode, force_reg (Pmode, op0));
+
+      emit_insn (gen_fxsave32 (op0));
+      return 0;
+
+    case IX86_BUILTIN_FXSAVE64:
+      arg0 = CALL_EXPR_ARG (exp, 0);
+      op0 = expand_normal (arg0);
+
+      if (GET_MODE (op0) != Pmode)
+	op0 = convert_to_mode (Pmode, op0, 1);
+      op0 = gen_rtx_MEM (Pmode, force_reg (Pmode, op0));
+
+      emit_insn (gen_fxsave64 (op0));
+      return 0;

Please merge these two in a similar way as XSAVE/XSAVEOPT case above, like:

icode = fcode == IX86_BUILTIN_FXSAVE32 ? CODE_FOR_xsave32 : CODE_FOR_xsave64;

pat = GEN_FCN (icode) (op0);
if (pat)
  emit_insn (pat);
return 0;

+(define_expand "fxsave32"
+  [(unspec_volatile [(match_operand 0 "memory_operand" "=m")]
+		    UNSPECV_FXSAVE32)]
+  "TARGET_FXSAVE")

You don't need any expanders, just use named patterns.

+(define_insn "*fxsave32"
+  [(unspec_volatile [(match_operand:P 0 "memory_operand" "=m")]
+		    UNSPECV_FXSAVE32)]
+  "TARGET_FXSAVE"
+  "fxsave\t%0"
+  [(set_attr "mode" "<MODE>")])
+
+(define_insn "*fxsave64"
+  [(unspec_volatile [(match_operand:P 0 "memory_operand" "=m")]
+		    UNSPECV_FXSAVE64)]
+  "TARGET_FXSAVE && TARGET_64BIT"
+  "fxsave64\t%0"
+  [(set_attr "mode" "<MODE>")])

You can use int_iterator here, like:

(define_int_iterator FXSAVE
	[UNSPEC_FXSAVE
	 (UNSPEC_FXSAVE64 "TARGET_64BIT")])

(define_int_attr fxsave
	[(UNSPEC_FXSAVE "fxsave")
	 (UNSPEC_FXSAVE64 "fxsave64")])

(define_insn "<fxsave>"
  [(unspec_volatile [(match_operand:P 0 "memory_operand" "=m")]
		    FXSAVE)]
  "TARGET_FXSAVE"
  "<fxsave>\t%0"
  [(set_attr "mode" "<MODE>")])

 ... define other patterns in a similar way ...

This way, all insns will be conditionally generated from the above
template using integer iterators. You will need to adapt the insn name
a bit (fxsave instead of fxsave32) to avoid introducing another
int_attribute.

Uros.