[i386] Introduce __readeflags () and __writeeflags () intrinsics.

On 05 Dec 09:01, Uros Bizjak wrote:
> On Thu, Dec 5, 2013 at 8:46 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> > +      emit_insn (gen_push (gen_rtx_REG (CCmode, FLAGS_REG)));
>
> The FLAGS_REG shuold be generated in an integer mode, appropriate for the push!
I suppose, `word_mode' is correct mode throughout these built-ins expanding.
 
> > +      emit_insn (gen_pop (target));
> > +      return target;
>
> Please note that "target" can be null, so you need to generate a
> register and move insn in this case. Please see many examples in
> i386.c expander code.
Fixed.
 
> > +    case IX86_BUILTIN_WRITE_FLAGS:
> > +      arg0 = CALL_EXPR_ARG (exp, 0);
> > +      emit_insn (gen_push (expand_normal (arg0)));
>
> This expand normal is too simple, you need to check predicate and move
> argument to a mode register. Also, there are many examples througout
> i386.c expanders.
Fixed using `push_operand' predicate.
 
> > +      emit_insn (gen_pop (gen_rtx_REG (CCmode, FLAGS_REG)));
>
> Again, FLAGS_REG should be generated in a correct mode. I wonder if
> "flags_reg_operand" correctly checks operand mode ...
Fixed.
> > +(define_insn "*popfl<mode>1"
>
> You can remove trailing 1 here ...
Fixed.

Bootstrapped. Tests still pass.
Changelogs untouched.

Is it ok?

--
Thanks, K

 gcc/config/i386/i386.c                        | 37 +++++++++++++++++++++++++
 gcc/config/i386/i386.md                       | 17 ++++++++++++
 gcc/config/i386/ia32intrin.h                  | 33 ++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/readeflags-1.c  | 40 +++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/writeeflags-1.c | 30 ++++++++++++++++++++
 5 files changed, 157 insertions(+)

On Thu, Dec 5, 2013 at 11:20 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:

>> > +      emit_insn (gen_push (gen_rtx_REG (CCmode, FLAGS_REG)));
>>
>> The FLAGS_REG shuold be generated in an integer mode, appropriate for the push!
> I suppose, `word_mode' is correct mode throughout these built-ins expanding.
>
>> > +      emit_insn (gen_pop (target));
>> > +      return target;
>>
>> Please note that "target" can be null, so you need to generate a
>> register and move insn in this case. Please see many examples in
>> i386.c expander code.
> Fixed.
>
>> > +    case IX86_BUILTIN_WRITE_FLAGS:
>> > +      arg0 = CALL_EXPR_ARG (exp, 0);
>> > +      emit_insn (gen_push (expand_normal (arg0)));
>>
>> This expand normal is too simple, you need to check predicate and move
>> argument to a mode register. Also, there are many examples througout
>> i386.c expanders.
> Fixed using `push_operand' predicate.

It should be general_no_elim_operand to match push patterns.

>> > +      emit_insn (gen_pop (gen_rtx_REG (CCmode, FLAGS_REG)));
>>
>> Again, FLAGS_REG should be generated in a correct mode. I wonder if
>> "flags_reg_operand" correctly checks operand mode ...
> Fixed.
>> > +(define_insn "*popfl<mode>1"
>>
>> You can remove trailing 1 here ...
> Fixed.
>
> Bootstrapped. Tests still pass.
> Changelogs untouched.
>
> Is it ok?

OK for mainline with two predicate changes to match push/pop patterns.

Thanks,
Uros.

>
> --
> Thanks, K
>
>  gcc/config/i386/i386.c                        | 37 +++++++++++++++++++++++++
>  gcc/config/i386/i386.md                       | 17 ++++++++++++
>  gcc/config/i386/ia32intrin.h                  | 33 ++++++++++++++++++++++
>  gcc/testsuite/gcc.target/i386/readeflags-1.c  | 40 +++++++++++++++++++++++++++
>  gcc/testsuite/gcc.target/i386/writeeflags-1.c | 30 ++++++++++++++++++++
>  5 files changed, 157 insertions(+)
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 21963bb..5bf358e 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -27909,6 +27909,10 @@ enum ix86_builtins
>    IX86_BUILTIN_CPU_IS,
>    IX86_BUILTIN_CPU_SUPPORTS,
>
> +  /* Read/write FLAGS register built-ins.  */
> +  IX86_BUILTIN_READ_FLAGS,
> +  IX86_BUILTIN_WRITE_FLAGS,
> +
>    IX86_BUILTIN_MAX
>  };
>
> @@ -29750,6 +29754,17 @@ ix86_init_mmx_sse_builtins (void)
>                UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PULONGLONG,
>                IX86_BUILTIN_ADDCARRYX64);
>
> +  /* Read/write FLAGS.  */
> +  def_builtin (~OPTION_MASK_ISA_64BIT, "__builtin_ia32_readeflags_u32",
> +               UNSIGNED_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
> +  def_builtin (OPTION_MASK_ISA_64BIT, "__builtin_ia32_readeflags_u64",
> +               UINT64_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
> +  def_builtin (~OPTION_MASK_ISA_64BIT, "__builtin_ia32_writeeflags_u32",
> +               VOID_FTYPE_UNSIGNED, IX86_BUILTIN_WRITE_FLAGS);
> +  def_builtin (OPTION_MASK_ISA_64BIT, "__builtin_ia32_writeeflags_u64",
> +               VOID_FTYPE_UINT64, IX86_BUILTIN_WRITE_FLAGS);
> +
> +
>    /* Add FMA4 multi-arg argument instructions */
>    for (i = 0, d = bdesc_multi_arg; i < ARRAY_SIZE (bdesc_multi_arg); i++, d++)
>      {
> @@ -33378,6 +33393,28 @@ addcarryx:
>        emit_insn (gen_rtx_SET (VOIDmode, target, pat));
>        return target;
>
> +    case IX86_BUILTIN_READ_FLAGS:
> +      emit_insn (gen_push (gen_rtx_REG (word_mode, FLAGS_REG)));
> +
> +      if (target == NULL_RTX
> +         || !register_operand (target, word_mode)

!nonimmediate operand (...)

> +         || GET_MODE (target) != word_mode)
> +       target = gen_reg_rtx (word_mode);
> +
> +      emit_insn (gen_pop (target));
> +      return target;
> +
> +    case IX86_BUILTIN_WRITE_FLAGS:
> +
> +      arg0 = CALL_EXPR_ARG (exp, 0);
> +      op0 = expand_normal (arg0);
> +      if (!push_operand (op0, word_mode))

!general_no_elim_operand (...)

> +       op0 = copy_to_mode_reg (word_mode, op0);
> +
> +      emit_insn (gen_push (op0));
> +      emit_insn (gen_pop (gen_rtx_REG (word_mode, FLAGS_REG)));
> +      return 0;
> +
>      case IX86_BUILTIN_GATHERSIV2DF:
>        icode = CODE_FOR_avx2_gathersiv2df;
>        goto gather_gen;
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 6976124..272fbdf 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -1714,6 +1714,23 @@
>    "pop{<imodesuffix>}\t%0"
>    [(set_attr "type" "pop")
>     (set_attr "mode" "<MODE>")])
> +
> +(define_insn "*pushfl<mode>"
> +  [(set (match_operand:DWIH 0 "push_operand" "=<")
> +       (match_operand:DWIH 1 "flags_reg_operand"))]
> +  ""
> +  "pushf{<imodesuffix>}"
> +  [(set_attr "type" "push")
> +   (set_attr "mode" "<MODE>")])
> +
> +(define_insn "*popfl<mode>"
> +  [(set (match_operand:DWIH 0 "flags_reg_operand")
> +       (match_operand:DWIH 1 "pop_operand" ">"))]
> +  ""
> +  "popf{<imodesuffix>}"
> +  [(set_attr "type" "pop")
> +   (set_attr "mode" "<MODE>")])
> +
>
>  ;; Move instructions.
>
> diff --git a/gcc/config/i386/ia32intrin.h b/gcc/config/i386/ia32intrin.h
> index b26dc46..65642e4 100644
> --- a/gcc/config/i386/ia32intrin.h
> +++ b/gcc/config/i386/ia32intrin.h
> @@ -238,6 +238,22 @@ __rorq (unsigned long long __X, int __C)
>    return (__X >> __C) | (__X << (64 - __C));
>  }
>
> +/* Read flags register */
> +extern __inline unsigned long long
> +__attribute__((__gnu_inline__, __always_inline__, __artificial__))
> +__readeflags (void)
> +{
> +  return __builtin_ia32_readeflags_u64 ();
> +}
> +
> +/* Write flags register */
> +extern __inline void
> +__attribute__((__gnu_inline__, __always_inline__, __artificial__))
> +__writeeflags (unsigned long long X)
> +{
> +  __builtin_ia32_writeeflags_u64 (X);
> +}
> +
>  #define _bswap64(a)            __bswapq(a)
>  #define _popcnt64(a)           __popcntq(a)
>  #define _lrotl(a,b)            __rolq((a), (b))
> @@ -245,6 +261,23 @@ __rorq (unsigned long long __X, int __C)
>  #else
>  #define _lrotl(a,b)            __rold((a), (b))
>  #define _lrotr(a,b)            __rord((a), (b))
> +
> +/* Read flags register */
> +extern __inline unsigned int
> +__attribute__((__gnu_inline__, __always_inline__, __artificial__))
> +__readeflags (void)
> +{
> +  return __builtin_ia32_readeflags_u32 ();
> +}
> +
> +/* Write flags register */
> +extern __inline void
> +__attribute__((__gnu_inline__, __always_inline__, __artificial__))
> +__writeeflags (unsigned int X)
> +{
> +  __builtin_ia32_writeeflags_u32 (X);
> +}
> +
>  #endif
>
>  #define _bit_scan_forward(a)   __bsfd(a)
> diff --git a/gcc/testsuite/gcc.target/i386/readeflags-1.c b/gcc/testsuite/gcc.target/i386/readeflags-1.c
> new file mode 100644
> index 0000000..6b2fa7e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/readeflags-1.c
> @@ -0,0 +1,40 @@
> +/* { dg-do run } */
> +/* { dg-options "-O0" } */
> +
> +#include <x86intrin.h>
> +
> +#ifdef __x86_64__
> +#define EFLAGS_TYPE unsigned long long int
> +#else
> +#define EFLAGS_TYPE unsigned int
> +#endif
> +
> +static EFLAGS_TYPE
> +readeflags_test (unsigned int a, unsigned int b)
> +{
> +  unsigned x = (a == b);
> +  return __readeflags ();
> +}
> +
> +int
> +main ()
> +{
> +  EFLAGS_TYPE flags;
> +
> +  flags = readeflags_test (100, 100);
> +
> +  if ((flags & 1) != 0)  /* Read CF */
> +    abort ();
> +
> +  flags = readeflags_test (100, 101);
> +
> +  if ((flags & 1) == 0)  /* Read CF */
> +    abort ();
> +
> +#ifdef DEBUG
> +    printf ("PASSED\n");
> +#endif
> +
> +  return 0;
> +}
> +
> diff --git a/gcc/testsuite/gcc.target/i386/writeeflags-1.c b/gcc/testsuite/gcc.target/i386/writeeflags-1.c
> new file mode 100644
> index 0000000..446840c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/writeeflags-1.c
> @@ -0,0 +1,30 @@
> +/* { dg-do run } */
> +/* { dg-options "-O0" } */
> +
> +#include <x86intrin.h>
> +
> +#ifdef __x86_64__
> +#define EFLAGS_TYPE unsigned long long int
> +#else
> +#define EFLAGS_TYPE unsigned int
> +#endif
> +
> +int
> +main ()
> +{
> +  EFLAGS_TYPE flags = 0xD7; /* 111010111b  */
> +
> +  __writeeflags (flags);
> +
> +  flags = __readeflags ();
> +
> +  if ((flags & 0xFF) != 0xD7)
> +    abort ();
> +
> +#ifdef DEBUG
> +    printf ("PASSED\n");
> +#endif
> +
> +  return 0;
> +}
> +