On Thu, Dec 5, 2013 at 8:46 AM, Kirill Yukhin <[email protected]> wrote:
>> Oh, no. We don't want assembly in this century ;)
> Whoops, sorry. I was trying to do it with minimal changes.
>
> I've implemented approach you proposed.
>
> Batch in the bottom.
> Bootstrapped. New tests pass.
>
> Is it ok now?
>
> ChangeLog/
> * config/i386/i386.c(IX86_BUILTIN_READ_FLAGS): New.
> (IX86_BUILTIN_WRITE_FLAGS): Ditto.
> (ix86_init_mmx_sse_builtins): Define
> __builtin_ia32_writeeflags_u32, __builtin_ia32_writeeflags_u64,
> __builtin_ia32_readeflags_u32, __builtin_ia32_readeflags_u64.
> (ix86_expand_builtin): Expand them.
> * config/i386/ia32intrin.h (__readeflags): New.
> (__writeeflags): Ditto.
> * gcc/config/i386/i386.md (*pushfl<mode>): Ditto.
> (*popfl<mode>1): Ditto.
>
> testsuite/ChangeLog same as initial mail.
>
> --
> Thanks, K
>
> gcc/config/i386/i386.c | 26 +++++++++++++++++
> 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, 146 insertions(+)
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 21963bb..f681346 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,17 @@ addcarryx:
> emit_insn (gen_rtx_SET (VOIDmode, target, pat));
> return target;
>
> + case IX86_BUILTIN_READ_FLAGS:
> + emit_insn (gen_push (gen_rtx_REG (CCmode, FLAGS_REG)));
The FLAGS_REG shuold be generated in an integer mode, appropriate for the push!
> + 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.
> + 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.
> + 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 ...
> + 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..1c6b06d 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>1"
You can remove trailing 1 here ...
Uros.
