On Wed, Oct 28, 2020 at 12:09:54PM -0300, Raoni Fassina Firmino wrote:
>         * builtins.c (expand_builtin_fegetround): New function.
>         (expand_builtin_feclear_feraise_except): New function.
>         (expand_builtin): Add cases for BUILT_IN_FEGETROUND,
>         BUILT_IN_FECLEAREXCEPT and BUILT_IN_FERAISEEXCEPT
>         * config/rs6000/rs6000.md (fegetroundsi): New pattern.
>         (feclearexceptsi): New Pattern.
>         (feraiseexceptsi): New Pattern.
>         * optabs.def (fegetround_optab): New optab.
>         (feclearexcept_optab): New optab.
>         (feraiseexcept_optab): New optab.
> 
> gcc/testsuite/ChangeLog:
> 
>         * gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c: New 
> test.
>         * gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c: New 
> test.
>         * gcc.target/powerpc/builtin-fegetround.c: New test.


> +/* Expand call EXP to the fegetround builtin (from C99 venv.h), returning the
> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  
> */

fenv.h

> +  rtx pat = GEN_FCN (icode) (target);
> +  if (! pat)
> +    return NULL_RTX;

No space after "!" (you do this more often; there are many bad examples
to follow, of course).

> +/* Expand call EXP to either feclearexcept or feraiseexcept builtins (from 
> C99
> +    fenv.h), returning the result and setting it in TARGET.  Otherwise return
> +    NULL_RTX on failure.  */

"f" and "N" should align with the "E".

> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -6565,6 +6565,87 @@
>    [(set_attr "type" "fpload")
>     (set_attr "length" "8")
>     (set_attr "isa" "*,p8v,p8v")])
> +
> +;; int __builtin_fegetround()

Well, give the arguments (or just their types) as well then?

> +(define_expand "fegetroundsi"
> +  [(use (match_operand:SI 0 "gpc_reg_operand"))]
> +  "TARGET_HARD_FLOAT"
> +{
> +  rtx tmp_df = gen_reg_rtx (DFmode);
> +  emit_insn (gen_rs6000_mffsl (tmp_df));
> +
> +  rtx tmp_di = simplify_gen_subreg (DImode, tmp_df, DFmode, 0);
> +  rtx tmp_di_2 = gen_reg_rtx (DImode);
> +  emit_insn (gen_anddi3 (tmp_di_2, tmp_di, GEN_INT (3)));
> +  rtx tmp_si = gen_reg_rtx (SImode);
> +  tmp_si = gen_lowpart (SImode, tmp_di_2);
> +  emit_move_insn (operands[0], tmp_si);
> +  DONE;
> +})

Okay.

> +;; int feclearexcept(int excepts)
> +;;
> +;; This expansion for the C99 function only works when excepts is a

(EXCEPTS)

> +;; constant known at compile time and specifies any one of
> +;; FE_INEXACT, FE_DIVBYZERO, FE_UNDERFLOW and FE_OVERFLOW flags.
> +;; It doesn't handle values out of range, and always returns 0.
> +;; Note that FE_INVALID is unsuported because it maps to more than

(unsupported)

> +;; one bit on FPSCR register.

s/on/of the/

> +;; Because of these restrictions, this only expands on the desired cases.

Maybe say in other cases you just get a call to libc?

> +(define_expand "feclearexceptsi"
> +  [(use (match_operand:SI 1 "const_int_operand" "n"))
> +   (set (match_operand:SI 0 "gpc_reg_operand")
> +     (const_int 0))]
> +  "TARGET_HARD_FLOAT"
> +{
> +  unsigned int fe = INTVAL (operands[1]);
> +  if (fe != (fe & 0x1e000000))
> +    FAIL;
> +
> +  if (fe & 0x02000000)  /* FE_INEXACT */
> +    emit_insn (gen_rs6000_mtfsb0 (gen_rtx_CONST_INT (SImode, 6)));
> +  if (fe & 0x04000000)  /* FE_DIVBYZERO */
> +    emit_insn (gen_rs6000_mtfsb0 (gen_rtx_CONST_INT (SImode, 5)));
> +  if (fe & 0x08000000)  /* FE_UNDERFLOW */
> +    emit_insn (gen_rs6000_mtfsb0 (gen_rtx_CONST_INT (SImode, 4)));
> +  if (fe & 0x10000000)  /* FE_OVERFLOW */
> +    emit_insn (gen_rs6000_mtfsb0 (gen_rtx_CONST_INT (SImode, 3)));
> +
> +  emit_move_insn (operands[0], const0_rtx);
> +  DONE;
> +})

Okay.

> +;; int feraiseexcept(int excepts)
> +;;
> +;; This expansion for the C99 function only works when excepts is a
> +;; constant known at compile time and specifies any one of
> +;; FE_INEXACT, FE_DIVBYZERO, FE_UNDERFLOW and FE_OVERFLOW flags.
> +;; It doesn't handle values out of range, and always returns 0.
> +;; Note that FE_INVALID is unsupported because it maps to more than
> +;; one bit on FPSCR register.
> +;; Because of these restrictions, this only expands on the desired cases.
> +(define_expand "feraiseexceptsi"
> +  [(use (match_operand:SI 1 "const_int_operand" "n"))
> +   (set (match_operand:SI 0 "gpc_reg_operand")
> +     (const_int 0))]
> +  "TARGET_HARD_FLOAT"
> +{
> +  unsigned int fe = INTVAL (operands[1]);
> +  if (fe != (fe & 0x1e000000))
> +    FAIL;
> +
> +  if (fe & 0x02000000)  /* FE_INEXACT */
> +    emit_insn (gen_rs6000_mtfsb1 (gen_rtx_CONST_INT (SImode, 6)));
> +  if (fe & 0x04000000)  /* FE_DIVBYZERO */
> +    emit_insn (gen_rs6000_mtfsb1 (gen_rtx_CONST_INT (SImode, 5)));
> +  if (fe & 0x08000000)  /* FE_UNDERFLOW */
> +    emit_insn (gen_rs6000_mtfsb1 (gen_rtx_CONST_INT (SImode, 4)));
> +  if (fe & 0x10000000)  /* FE_OVERFLOW */
> +    emit_insn (gen_rs6000_mtfsb1 (gen_rtx_CONST_INT (SImode, 3)));
> +
> +  emit_move_insn (operands[0], const0_rtx);
> +  DONE;
> +})

For raising invalid we could perhaps set VXSOFT, but is that okay for
other libcs than just glibc?  So that can be a future improvement if it
turns out to be useful, as we discussed elsewhere.

> +@cindex @code{fegetround@var{m}} instruction pattern
> +@item @samp{fegetround@var{m}}
> +Store the current machine floating-point rounding mode into operand 0.
> +Operand 0 has mode @var{m}, which is scalar.  This pattern is used to
> +implement the @code{fegetround} builtin function from the ISO C99
> +standard.

s/builtin //

For all those standard functions we have a __builtin_* variant as well,
so no need to mention it here I think.

> +@cindex @code{feraiseexcept@var{m}} instruction pattern
> +@item @samp{feclearexcept@var{m}}
> +@item @samp{feraiseexcept@var{m}}
> +Clears or raises the supported machine floating-point exceptions
> +represented by the bits in operand 1.  Error status is stored as
> +nonzero value in operand 0.  Both operands have mode @var{m}, which is
> +a scalar.  This patterns are used to implement the

"These patterns"


With those tweaks, the rs6000 part of the patch is okay for trunk.
Thanks!  (The generic part looks fine to me as well, but I am not
maintainer of that.)


Segher

Reply via email to