[PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Raoni Fassina Firmino]

I am repeating the "changelog" from v3 and v4 here because v4 and v5
have just minor changes since v3.

Changes since v4[1]:
  - Fixed more spelling and code style.
  - Add more clarification on  comments for feraiseexcept and
    feclearexcept expands;

Changes since v3[2]:
  - Fixed fegetround bug on powerpc64 (big endian) that Segher
    spotted;

Changes since v2[3]:
  - Added documentation for the new optabs;
  - Remove use of non portable __builtin_clz;
  - Changed feclearexcept and feraiseexcept to accept all 4 valid
    flags at the same time and added more test for that case;
  - Extended feclearexcept and feraiseexcept testcases to match
    accepting multiple flags;
  - Fixed builtin-feclearexcept-feraiseexcept-2.c testcase comparison
    after feclearexcept tests;
  - Updated commit message to reflect change in feclearexcept and
    feraiseexcept from the glibc conterpart;
  - Fixed English spelling and typos;
  - Fixed code-style;
  - Changed subject line tag to make clear it is not just rs6000 code.

Tested on top of master (23ac7a009ecfeec3eab79136abed8aac9768b458)
on the following plataforms with no regression:
  - powerpc64le-linux-gnu (Power 9)
  - powerpc64le-linux-gnu (Power 8)
  - powerpc64-linux-gnu (Power 8)
  - powerpc-linux-gnu (Power 8)

Documentation changes tested on x86_64-redhat-linux.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557349.html
[2] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557109.html
[3] https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553297.html

---- 8< ----

This optimizations were originally in glibc, but was removed
and suggested that they were a good fit as gcc builtins[1].

feclearexcept and feraiseexcept were extended (in comparison to the
glibc version) to accept any combination of the accepted flags, not
limited to just one flag bit at a time anymore.

The associated bugreport: PR target/94193

[1] https://sourceware.org/legacy-ml/libc-alpha/2020-03/msg00047.html
    https://sourceware.org/legacy-ml/libc-alpha/2020-03/msg00080.html

2020-08-13  Raoni Fassina Firmino  <raoni@linux.ibm.com>

gcc/ChangeLog:

        * 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.

Signed-off-by: Raoni Fassina Firmino <raoni@linux.ibm.com>
---
 gcc/builtins.c                                |  76 +++++++
 gcc/config/rs6000/rs6000.md                   |  83 +++++++
 gcc/doc/md.texi                               |  17 ++
 gcc/optabs.def                                |   4 +
 .../builtin-feclearexcept-feraiseexcept-1.c   |  76 +++++++
 .../builtin-feclearexcept-feraiseexcept-2.c   | 203 ++++++++++++++++++
 .../gcc.target/powerpc/builtin-fegetround.c   |  36 ++++
 7 files changed, 495 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index da25343beb1..4d80f34a110 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -116,6 +116,9 @@ static rtx expand_builtin_mathfn_3 (tree, rtx, rtx);
 static rtx expand_builtin_mathfn_ternary (tree, rtx, rtx);
 static rtx expand_builtin_interclass_mathfn (tree, rtx);
 static rtx expand_builtin_sincos (tree);
+static rtx expand_builtin_fegetround (tree, rtx, machine_mode);
+static rtx expand_builtin_feclear_feraise_except (tree, rtx, machine_mode,
+						  optab);
 static rtx expand_builtin_cexpi (tree, rtx);
 static rtx expand_builtin_int_roundingfn (tree, rtx);
 static rtx expand_builtin_int_roundingfn_2 (tree, rtx);
@@ -2893,6 +2896,59 @@ expand_builtin_sincos (tree exp)
   return const0_rtx;
 }
 
+/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
+   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
+static rtx
+expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
+{
+  if (!validate_arglist (exp, VOID_TYPE))
+    return NULL_RTX;
+
+  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
+  if (icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
+  if (target == 0
+      || GET_MODE (target) != target_mode
+      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
+    target = gen_reg_rtx (target_mode);
+
+  rtx pat = GEN_FCN (icode) (target);
+  if (!pat)
+    return NULL_RTX;
+  emit_insn (pat);
+
+  return target;
+}
+
+/* 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.  */
+static rtx
+expand_builtin_feclear_feraise_except (tree exp, rtx target,
+				       machine_mode target_mode, optab op_optab)
+{
+  if (!validate_arglist (exp, INTEGER_TYPE, VOID_TYPE))
+    return NULL_RTX;
+  rtx op0 = expand_normal (CALL_EXPR_ARG (exp, 0));
+
+  insn_code icode = direct_optab_handler (op_optab, SImode);
+  if (icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
+  if (target == 0
+      || GET_MODE (target) != target_mode
+      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
+    target = gen_reg_rtx (target_mode);
+
+  rtx pat = GEN_FCN (icode) (target, op0);
+  if (!pat)
+    return NULL_RTX;
+  emit_insn (pat);
+
+  return target;
+}
+
 /* Expand a call to the internal cexpi builtin to the sincos math function.
    EXP is the expression that is a call to the builtin function; if convenient,
    the result should be placed in TARGET.  */
@@ -8919,6 +8975,26 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
 	return target;
       break;
 
+    case BUILT_IN_FEGETROUND:
+      target = expand_builtin_fegetround (exp, target, target_mode);
+      if (target)
+	return target;
+      break;
+
+    case BUILT_IN_FECLEAREXCEPT:
+      target = expand_builtin_feclear_feraise_except (exp, target, target_mode,
+						      feclearexcept_optab);
+      if (target)
+	return target;
+      break;
+
+    case BUILT_IN_FERAISEEXCEPT:
+      target = expand_builtin_feclear_feraise_except (exp, target, target_mode,
+						      feraiseexcept_optab);
+      if (target)
+	return target;
+      break;
+
     case BUILT_IN_APPLY_ARGS:
       return expand_builtin_apply_args ();
 
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index dc060143104..bb6fa98676d 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -6565,6 +6565,89 @@
   [(set_attr "type" "fpload")
    (set_attr "length" "8")
    (set_attr "isa" "*,p8v,p8v")])
+
+;; int __builtin_fegetround(void)
+(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;
+})
+
+;; int feclearexcept(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 of the FPSCR register.
+;; Because of these restrictions, this only expands on the desired
+;; cases and fallback to a call to libc on any other case.
+(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;
+})
+
+;; 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 of the FPSCR register.
+;; Because of these restrictions, this only expands on the desired
+;; cases and fallback to a call to libc on any other case.
+(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;
+})
 \f
 ;; Define the TImode operations that can be done in a small number
 ;; of instructions.  The & constraints are to prevent the register
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 2b462869437..27f0f83cbf4 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5930,6 +5930,23 @@ mode @var{m}, which is a scalar or vector floating-point mode.
 
 This pattern is not allowed to @code{FAIL}.
 
+@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} function from the ISO C99 standard.
+
+@cindex @code{feclearexcept@var{m}} instruction pattern
+@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.  These patterns are used to implement the
+@code{feclearexcept} and @code{feraiseexcept} functions from the ISO
+C99 standard.
+
 @cindex @code{exp@var{m}2} instruction pattern
 @item @samp{exp@var{m}2}
 Raise e (the base of natural logarithms) to the power of operand 1
diff --git a/gcc/optabs.def b/gcc/optabs.def
index 78409aa1453..987ee0f79dc 100644
--- a/gcc/optabs.def
+++ b/gcc/optabs.def
@@ -318,6 +318,10 @@ OPTAB_D (sinh_optab, "sinh$a2")
 OPTAB_D (tan_optab, "tan$a2")
 OPTAB_D (tanh_optab, "tanh$a2")
 
+OPTAB_D (fegetround_optab, "fegetround$a")
+OPTAB_D (feclearexcept_optab, "feclearexcept$a")
+OPTAB_D (feraiseexcept_optab, "feraiseexcept$a")
+
 /* C99 implementations of fmax/fmin.  */
 OPTAB_D (fmax_optab, "fmax$a3")
 OPTAB_D (fmin_optab, "fmin$a3")
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c
new file mode 100644
index 00000000000..c9d5b5cefc0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c
@@ -0,0 +1,76 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fenv_exceptions } */
+/* { dg-options "-lm -fno-builtin" } */
+
+/* This testcase ensures that the builtins expand with the matching arguments
+ * or otherwise fallback gracefully to a function call, and don't ICE during
+ * compilation.
+ * "-fno-builtin" option is used to enable calls to libc implementation of the
+ * gcc builtins tested when not using __builtin_ prefix. */
+
+#include <fenv.h>
+
+int
+main ()
+{
+  int   rsi = 0;
+  long  rsl = 0;
+  short rss = 0;
+  char  rsc = 0;
+
+  unsigned int   rui = 0;
+  unsigned long  rul = 0;
+  unsigned short rus = 0;
+  unsigned char  ruc = 0;
+
+  int e = FE_DIVBYZERO;
+
+  __builtin_feclearexcept(e);                          // CALL
+  __builtin_feclearexcept(FE_ALL_EXCEPT);              // CALL
+  __builtin_feclearexcept(FE_INVALID);                 // CALL
+  __builtin_feclearexcept(FE_INVALID | FE_INEXACT);    // CALL
+
+  __builtin_feclearexcept(FE_INEXACT | FE_DIVBYZERO |
+                          FE_UNDERFLOW | FE_OVERFLOW);  // EXPAND
+  __builtin_feclearexcept(FE_INEXACT | FE_OVERFLOW);    // EXPAND
+  __builtin_feclearexcept(FE_INEXACT);                  // EXPAND
+  __builtin_feclearexcept(FE_DIVBYZERO);                // EXPAND
+  __builtin_feclearexcept(FE_UNDERFLOW);                // EXPAND
+  __builtin_feclearexcept(FE_OVERFLOW);                 // EXPAND
+  __builtin_feclearexcept(0);                           // EXPAND
+
+  rsi = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  rsl = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  rss = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  rsc = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  rui = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  rul = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  rus = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+  ruc = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
+
+
+  __builtin_feraiseexcept(e);                          // CALL
+  __builtin_feraiseexcept(FE_ALL_EXCEPT);              // CALL
+  __builtin_feraiseexcept(FE_INVALID);                 // CALL
+  __builtin_feraiseexcept(FE_INVALID | FE_INEXACT);    // CALL
+
+  __builtin_feraiseexcept(FE_INEXACT | FE_DIVBYZERO |
+                          FE_UNDERFLOW | FE_OVERFLOW);  // EXPAND
+  __builtin_feraiseexcept(FE_INEXACT | FE_OVERFLOW);    // EXPAND
+  __builtin_feraiseexcept(FE_INEXACT);                  // EXPAND
+  __builtin_feraiseexcept(FE_DIVBYZERO);                // EXPAND
+  __builtin_feraiseexcept(FE_UNDERFLOW);                // EXPAND
+  __builtin_feraiseexcept(FE_OVERFLOW);                 // EXPAND
+  __builtin_feraiseexcept(0);                           // EXPAND
+
+  rsi = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  rsl = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  rss = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  rsc = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  rui = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  rul = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  rus = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+  ruc = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c
new file mode 100644
index 00000000000..e1bf8091014
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c
@@ -0,0 +1,203 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fenv_exceptions } */
+/* { dg-options "-lm -fno-builtin" } */
+
+/* This testcase ensures that the builtins are correctly expanded and match the
+ * expected result.
+ * "-fno-builtin" option is used to enable calls to libc implementation of the
+ * gcc builtins tested when not using __builtin_ prefix. */
+
+#include <fenv.h>
+
+#ifdef DEBUG
+#include <stdio.h>
+#define INFO(...) printf(__VA_ARGS__)
+#define FAIL(v, e, x, s, f) \
+        printf("ERROR [l %d] testing %s(%x): %s returned %x," \
+               " expecected %x\n", __LINE__, s, x, f, v, e)
+#else
+void abort (void);
+#define INFO(...)
+#define FAIL(v, e, x, s, f) abort()
+#endif
+
+int
+main ()
+{
+  char *s = 0;
+  int e = 0;
+  int raised = 0;
+
+  s = "FE_ALL_EXCEPT";
+  e = FE_ALL_EXCEPT;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_ALL_EXCEPT);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_ALL_EXCEPT);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "NONE";
+  e = 0;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(0);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(0);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_DIVBYZERO";
+  e = FE_DIVBYZERO;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_DIVBYZERO);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_DIVBYZERO);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_INEXACT";
+  e = FE_INEXACT;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_INEXACT);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_INEXACT);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_OVERFLOW";
+  e = FE_OVERFLOW;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_OVERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_OVERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_UNDERFLOW";
+  e = FE_UNDERFLOW;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_UNDERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_UNDERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_INVALID";
+  e = FE_INVALID;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_INVALID);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_INVALID);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_INVALID | FE_INEXACT";
+  e = FE_INVALID | FE_INEXACT;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_INVALID | FE_INEXACT);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_INVALID | FE_INEXACT);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_INEXACT | FE_OVERFLOW";
+  e = FE_INEXACT | FE_OVERFLOW;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_INEXACT | FE_OVERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_INEXACT | FE_OVERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+
+  s = "FE_INEXACT | FE_DIVBYZERO | FE_UNDERFLOW | FE_OVERFLOW";
+  e = FE_INEXACT | FE_DIVBYZERO | FE_UNDERFLOW | FE_OVERFLOW;
+  INFO("test: %s(%x)\n", s, e);
+
+  feclearexcept(FE_ALL_EXCEPT);
+  __builtin_feraiseexcept(FE_INEXACT | FE_DIVBYZERO |
+                          FE_UNDERFLOW | FE_OVERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != e)
+    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
+
+  feraiseexcept(FE_ALL_EXCEPT);
+  __builtin_feclearexcept(FE_INEXACT | FE_DIVBYZERO |
+                          FE_UNDERFLOW | FE_OVERFLOW);
+  raised = fetestexcept(FE_ALL_EXCEPT);
+  if (raised != (FE_ALL_EXCEPT & ~e))
+    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c b/gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c
new file mode 100644
index 00000000000..502ddf30ae2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c
@@ -0,0 +1,36 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fenv_exceptions } */
+/* { dg-options "-lm -fno-builtin" } */
+
+/* This testcase ensures that the builtins is correctly expanded and match the
+ * expected result from the standard function.
+ * "-fno-builtin" option is used to enable calls to libc implementation of the
+ * gcc builtins tested when not using __builtin_ prefix. */
+
+#include <fenv.h>
+
+#ifdef DEBUG
+#include <stdio.h>
+#define FAIL(v, e) printf("ERROR, __builtin_fegetround() returned %d," \
+                          " not the expecected value %d\n", v, e);
+#else
+void abort (void);
+#define FAIL(v, e) abort()
+#endif
+
+int
+main ()
+{
+  int i, rounding, expected;
+  const int rm[] = {FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD, FE_DOWNWARD};
+  for (i = 0; i < sizeof(rm); i++)
+    {
+      fesetround(rm[i]);
+      rounding = __builtin_fegetround();
+      expected = fegetround();
+      if (rounding != expected)
+        FAIL(rounding, expected);
+    }
+
+  return 0;
+}
-- 
2.26.2

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Richard Biener]

On Tue, 3 Nov 2020, Raoni Fassina Firmino wrote:

> I am repeating the "changelog" from v3 and v4 here because v4 and v5
> have just minor changes since v3.
> 
> Changes since v4[1]:
>   - Fixed more spelling and code style.
>   - Add more clarification on  comments for feraiseexcept and
>     feclearexcept expands;
> 
> Changes since v3[2]:
>   - Fixed fegetround bug on powerpc64 (big endian) that Segher
>     spotted;
> 
> Changes since v2[3]:
>   - Added documentation for the new optabs;
>   - Remove use of non portable __builtin_clz;
>   - Changed feclearexcept and feraiseexcept to accept all 4 valid
>     flags at the same time and added more test for that case;
>   - Extended feclearexcept and feraiseexcept testcases to match
>     accepting multiple flags;
>   - Fixed builtin-feclearexcept-feraiseexcept-2.c testcase comparison
>     after feclearexcept tests;
>   - Updated commit message to reflect change in feclearexcept and
>     feraiseexcept from the glibc conterpart;
>   - Fixed English spelling and typos;
>   - Fixed code-style;
>   - Changed subject line tag to make clear it is not just rs6000 code.
> 
> Tested on top of master (23ac7a009ecfeec3eab79136abed8aac9768b458)
> on the following plataforms with no regression:
>   - powerpc64le-linux-gnu (Power 9)
>   - powerpc64le-linux-gnu (Power 8)
>   - powerpc64-linux-gnu (Power 8)
>   - powerpc-linux-gnu (Power 8)
> 
> Documentation changes tested on x86_64-redhat-linux.
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557349.html
> [2] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557109.html
> [3] https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553297.html
> 
> ---- 8< ----
> 
> This optimizations were originally in glibc, but was removed
> and suggested that they were a good fit as gcc builtins[1].
> 
> feclearexcept and feraiseexcept were extended (in comparison to the
> glibc version) to accept any combination of the accepted flags, not
> limited to just one flag bit at a time anymore.
> 
> The associated bugreport: PR target/94193
> 
> [1] https://sourceware.org/legacy-ml/libc-alpha/2020-03/msg00047.html
>     https://sourceware.org/legacy-ml/libc-alpha/2020-03/msg00080.html
> 
> 2020-08-13  Raoni Fassina Firmino  <raoni@linux.ibm.com>
> 
> gcc/ChangeLog:
> 
>         * 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.
> 
> Signed-off-by: Raoni Fassina Firmino <raoni@linux.ibm.com>
> ---
>  gcc/builtins.c                                |  76 +++++++
>  gcc/config/rs6000/rs6000.md                   |  83 +++++++
>  gcc/doc/md.texi                               |  17 ++
>  gcc/optabs.def                                |   4 +
>  .../builtin-feclearexcept-feraiseexcept-1.c   |  76 +++++++
>  .../builtin-feclearexcept-feraiseexcept-2.c   | 203 ++++++++++++++++++
>  .../gcc.target/powerpc/builtin-fegetround.c   |  36 ++++
>  7 files changed, 495 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c
> 
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index da25343beb1..4d80f34a110 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -116,6 +116,9 @@ static rtx expand_builtin_mathfn_3 (tree, rtx, rtx);
>  static rtx expand_builtin_mathfn_ternary (tree, rtx, rtx);
>  static rtx expand_builtin_interclass_mathfn (tree, rtx);
>  static rtx expand_builtin_sincos (tree);
> +static rtx expand_builtin_fegetround (tree, rtx, machine_mode);
> +static rtx expand_builtin_feclear_feraise_except (tree, rtx, machine_mode,
> +						  optab);
>  static rtx expand_builtin_cexpi (tree, rtx);
>  static rtx expand_builtin_int_roundingfn (tree, rtx);
>  static rtx expand_builtin_int_roundingfn_2 (tree, rtx);
> @@ -2893,6 +2896,59 @@ expand_builtin_sincos (tree exp)
>    return const0_rtx;
>  }
>  
> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
> +static rtx
> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
> +{
> +  if (!validate_arglist (exp, VOID_TYPE))
> +    return NULL_RTX;
> +
> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
> +  if (icode == CODE_FOR_nothing)
> +    return NULL_RTX;
> +
> +  if (target == 0
> +      || GET_MODE (target) != target_mode
> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
> +    target = gen_reg_rtx (target_mode);
> +
> +  rtx pat = GEN_FCN (icode) (target);
> +  if (!pat)
> +    return NULL_RTX;
> +  emit_insn (pat);

I think you need to verify whether the expansion ended up in 'target'
and otherwise emit a move since usually 'target' is just a hint.

> +
> +  return target;
> +}
> +
> +/* 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.  */
> +static rtx
> +expand_builtin_feclear_feraise_except (tree exp, rtx target,
> +				       machine_mode target_mode, optab op_optab)
> +{
> +  if (!validate_arglist (exp, INTEGER_TYPE, VOID_TYPE))
> +    return NULL_RTX;
> +  rtx op0 = expand_normal (CALL_EXPR_ARG (exp, 0));
> +
> +  insn_code icode = direct_optab_handler (op_optab, SImode);
> +  if (icode == CODE_FOR_nothing)
> +    return NULL_RTX;
> +
> +  if (target == 0
> +      || GET_MODE (target) != target_mode
> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
> +    target = gen_reg_rtx (target_mode);
> +
> +  rtx pat = GEN_FCN (icode) (target, op0);
> +  if (!pat)
> +    return NULL_RTX;
> +  emit_insn (pat);

Likewise.

> +  return target;
> +}
> +
>  /* Expand a call to the internal cexpi builtin to the sincos math function.
>     EXP is the expression that is a call to the builtin function; if convenient,
>     the result should be placed in TARGET.  */
> @@ -8919,6 +8975,26 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
>  	return target;
>        break;
>  
> +    case BUILT_IN_FEGETROUND:
> +      target = expand_builtin_fegetround (exp, target, target_mode);
> +      if (target)
> +	return target;
> +      break;
> +
> +    case BUILT_IN_FECLEAREXCEPT:
> +      target = expand_builtin_feclear_feraise_except (exp, target, target_mode,
> +						      feclearexcept_optab);
> +      if (target)
> +	return target;
> +      break;
> +
> +    case BUILT_IN_FERAISEEXCEPT:
> +      target = expand_builtin_feclear_feraise_except (exp, target, target_mode,
> +						      feraiseexcept_optab);
> +      if (target)
> +	return target;
> +      break;
> +
>      case BUILT_IN_APPLY_ARGS:
>        return expand_builtin_apply_args ();
>  
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index dc060143104..bb6fa98676d 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -6565,6 +6565,89 @@
>    [(set_attr "type" "fpload")
>     (set_attr "length" "8")
>     (set_attr "isa" "*,p8v,p8v")])
> +
> +;; int __builtin_fegetround(void)
> +(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;
> +})
> +
> +;; int feclearexcept(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 of the FPSCR register.
> +;; Because of these restrictions, this only expands on the desired
> +;; cases and fallback to a call to libc on any other case.
> +(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;
> +})
> +
> +;; 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 of the FPSCR register.
> +;; Because of these restrictions, this only expands on the desired
> +;; cases and fallback to a call to libc on any other case.
> +(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;
> +})
>  \f
>  ;; Define the TImode operations that can be done in a small number
>  ;; of instructions.  The & constraints are to prevent the register
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 2b462869437..27f0f83cbf4 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -5930,6 +5930,23 @@ mode @var{m}, which is a scalar or vector floating-point mode.
>  
>  This pattern is not allowed to @code{FAIL}.
>  
> +@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} function from the ISO C99 standard.

I think this needs to elaborate on the format of the "rounding mode".

AFAICS you do nothing to marshall with the actually used libc
implementation which AFAIU can choose arbitrary values for
the FE_* macros.  I'm not sure we require the compiler to be
configured for one specific C library and for example require
matching FE_* macro definitions for all uses of the built
compiler.

For the patch at hand you seem to assume the libc "format"
matches the hardware one (which would of course be reasonable).

Does that actually hold up when looking at libcs other than 
glibc supporting powerpc?

If all of these are non-issues then the middle-end pices look OK.
If we need any such "translation" layer then I guess we need
to either have additional operands to the optabs specifying
all of the FE_* values relevant for the respective call or
provide a side-channel (target hook) to implement the
translation on the expansion side.

> +
> +@cindex @code{feclearexcept@var{m}} instruction pattern
> +@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.  These patterns are used to implement the
> +@code{feclearexcept} and @code{feraiseexcept} functions from the ISO
> +C99 standard.
> +

Same concerns as above of course.

Now, I wonder whether _GCC_ should provide the FE_* macros, thus
move (parts of) fenv.h to GCC like we do for stdint.h?

Richard.

>  @cindex @code{exp@var{m}2} instruction pattern
>  @item @samp{exp@var{m}2}
>  Raise e (the base of natural logarithms) to the power of operand 1
> diff --git a/gcc/optabs.def b/gcc/optabs.def
> index 78409aa1453..987ee0f79dc 100644
> --- a/gcc/optabs.def
> +++ b/gcc/optabs.def
> @@ -318,6 +318,10 @@ OPTAB_D (sinh_optab, "sinh$a2")
>  OPTAB_D (tan_optab, "tan$a2")
>  OPTAB_D (tanh_optab, "tanh$a2")
>  
> +OPTAB_D (fegetround_optab, "fegetround$a")
> +OPTAB_D (feclearexcept_optab, "feclearexcept$a")
> +OPTAB_D (feraiseexcept_optab, "feraiseexcept$a")
> +
>  /* C99 implementations of fmax/fmin.  */
>  OPTAB_D (fmax_optab, "fmax$a3")
>  OPTAB_D (fmin_optab, "fmin$a3")
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c
> new file mode 100644
> index 00000000000..c9d5b5cefc0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-1.c
> @@ -0,0 +1,76 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target fenv_exceptions } */
> +/* { dg-options "-lm -fno-builtin" } */
> +
> +/* This testcase ensures that the builtins expand with the matching arguments
> + * or otherwise fallback gracefully to a function call, and don't ICE during
> + * compilation.
> + * "-fno-builtin" option is used to enable calls to libc implementation of the
> + * gcc builtins tested when not using __builtin_ prefix. */
> +
> +#include <fenv.h>
> +
> +int
> +main ()
> +{
> +  int   rsi = 0;
> +  long  rsl = 0;
> +  short rss = 0;
> +  char  rsc = 0;
> +
> +  unsigned int   rui = 0;
> +  unsigned long  rul = 0;
> +  unsigned short rus = 0;
> +  unsigned char  ruc = 0;
> +
> +  int e = FE_DIVBYZERO;
> +
> +  __builtin_feclearexcept(e);                          // CALL
> +  __builtin_feclearexcept(FE_ALL_EXCEPT);              // CALL
> +  __builtin_feclearexcept(FE_INVALID);                 // CALL
> +  __builtin_feclearexcept(FE_INVALID | FE_INEXACT);    // CALL
> +
> +  __builtin_feclearexcept(FE_INEXACT | FE_DIVBYZERO |
> +                          FE_UNDERFLOW | FE_OVERFLOW);  // EXPAND
> +  __builtin_feclearexcept(FE_INEXACT | FE_OVERFLOW);    // EXPAND
> +  __builtin_feclearexcept(FE_INEXACT);                  // EXPAND
> +  __builtin_feclearexcept(FE_DIVBYZERO);                // EXPAND
> +  __builtin_feclearexcept(FE_UNDERFLOW);                // EXPAND
> +  __builtin_feclearexcept(FE_OVERFLOW);                 // EXPAND
> +  __builtin_feclearexcept(0);                           // EXPAND
> +
> +  rsi = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  rsl = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  rss = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  rsc = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  rui = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  rul = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  rus = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +  ruc = __builtin_feclearexcept(FE_DIVBYZERO);  // EXPAND
> +
> +
> +  __builtin_feraiseexcept(e);                          // CALL
> +  __builtin_feraiseexcept(FE_ALL_EXCEPT);              // CALL
> +  __builtin_feraiseexcept(FE_INVALID);                 // CALL
> +  __builtin_feraiseexcept(FE_INVALID | FE_INEXACT);    // CALL
> +
> +  __builtin_feraiseexcept(FE_INEXACT | FE_DIVBYZERO |
> +                          FE_UNDERFLOW | FE_OVERFLOW);  // EXPAND
> +  __builtin_feraiseexcept(FE_INEXACT | FE_OVERFLOW);    // EXPAND
> +  __builtin_feraiseexcept(FE_INEXACT);                  // EXPAND
> +  __builtin_feraiseexcept(FE_DIVBYZERO);                // EXPAND
> +  __builtin_feraiseexcept(FE_UNDERFLOW);                // EXPAND
> +  __builtin_feraiseexcept(FE_OVERFLOW);                 // EXPAND
> +  __builtin_feraiseexcept(0);                           // EXPAND
> +
> +  rsi = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  rsl = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  rss = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  rsc = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  rui = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  rul = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  rus = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +  ruc = __builtin_feraiseexcept(FE_DIVBYZERO);  // EXPAND
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c
> new file mode 100644
> index 00000000000..e1bf8091014
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin-feclearexcept-feraiseexcept-2.c
> @@ -0,0 +1,203 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target fenv_exceptions } */
> +/* { dg-options "-lm -fno-builtin" } */
> +
> +/* This testcase ensures that the builtins are correctly expanded and match the
> + * expected result.
> + * "-fno-builtin" option is used to enable calls to libc implementation of the
> + * gcc builtins tested when not using __builtin_ prefix. */
> +
> +#include <fenv.h>
> +
> +#ifdef DEBUG
> +#include <stdio.h>
> +#define INFO(...) printf(__VA_ARGS__)
> +#define FAIL(v, e, x, s, f) \
> +        printf("ERROR [l %d] testing %s(%x): %s returned %x," \
> +               " expecected %x\n", __LINE__, s, x, f, v, e)
> +#else
> +void abort (void);
> +#define INFO(...)
> +#define FAIL(v, e, x, s, f) abort()
> +#endif
> +
> +int
> +main ()
> +{
> +  char *s = 0;
> +  int e = 0;
> +  int raised = 0;
> +
> +  s = "FE_ALL_EXCEPT";
> +  e = FE_ALL_EXCEPT;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_ALL_EXCEPT);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_ALL_EXCEPT);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "NONE";
> +  e = 0;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(0);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(0);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_DIVBYZERO";
> +  e = FE_DIVBYZERO;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_DIVBYZERO);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_DIVBYZERO);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_INEXACT";
> +  e = FE_INEXACT;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_INEXACT);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_INEXACT);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_OVERFLOW";
> +  e = FE_OVERFLOW;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_OVERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_OVERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_UNDERFLOW";
> +  e = FE_UNDERFLOW;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_UNDERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_UNDERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_INVALID";
> +  e = FE_INVALID;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_INVALID);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_INVALID);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_INVALID | FE_INEXACT";
> +  e = FE_INVALID | FE_INEXACT;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_INVALID | FE_INEXACT);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_INVALID | FE_INEXACT);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_INEXACT | FE_OVERFLOW";
> +  e = FE_INEXACT | FE_OVERFLOW;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_INEXACT | FE_OVERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_INEXACT | FE_OVERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +
> +  s = "FE_INEXACT | FE_DIVBYZERO | FE_UNDERFLOW | FE_OVERFLOW";
> +  e = FE_INEXACT | FE_DIVBYZERO | FE_UNDERFLOW | FE_OVERFLOW;
> +  INFO("test: %s(%x)\n", s, e);
> +
> +  feclearexcept(FE_ALL_EXCEPT);
> +  __builtin_feraiseexcept(FE_INEXACT | FE_DIVBYZERO |
> +                          FE_UNDERFLOW | FE_OVERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != e)
> +    FAIL(raised, e, e, s, "__builtin_feraiseexcept");
> +
> +  feraiseexcept(FE_ALL_EXCEPT);
> +  __builtin_feclearexcept(FE_INEXACT | FE_DIVBYZERO |
> +                          FE_UNDERFLOW | FE_OVERFLOW);
> +  raised = fetestexcept(FE_ALL_EXCEPT);
> +  if (raised != (FE_ALL_EXCEPT & ~e))
> +    FAIL(raised, FE_ALL_EXCEPT & ~e, e, s, "__builtin_feclearexcept");
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c b/gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c
> new file mode 100644
> index 00000000000..502ddf30ae2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtin-fegetround.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target fenv_exceptions } */
> +/* { dg-options "-lm -fno-builtin" } */
> +
> +/* This testcase ensures that the builtins is correctly expanded and match the
> + * expected result from the standard function.
> + * "-fno-builtin" option is used to enable calls to libc implementation of the
> + * gcc builtins tested when not using __builtin_ prefix. */
> +
> +#include <fenv.h>
> +
> +#ifdef DEBUG
> +#include <stdio.h>
> +#define FAIL(v, e) printf("ERROR, __builtin_fegetround() returned %d," \
> +                          " not the expecected value %d\n", v, e);
> +#else
> +void abort (void);
> +#define FAIL(v, e) abort()
> +#endif
> +
> +int
> +main ()
> +{
> +  int i, rounding, expected;
> +  const int rm[] = {FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD, FE_DOWNWARD};
> +  for (i = 0; i < sizeof(rm); i++)
> +    {
> +      fesetround(rm[i]);
> +      rounding = __builtin_fegetround();
> +      expected = fegetround();
> +      if (rounding != expected)
> +        FAIL(rounding, expected);
> +    }
> +
> +  return 0;
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Raoni Fassina Firmino]

On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
> > +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
> > +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
> > +static rtx
> > +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
> > +{
> > +  if (!validate_arglist (exp, VOID_TYPE))
> > +    return NULL_RTX;
> > +
> > +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
> > +  if (icode == CODE_FOR_nothing)
> > +    return NULL_RTX;
> > +
> > +  if (target == 0
> > +      || GET_MODE (target) != target_mode
> > +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
> > +    target = gen_reg_rtx (target_mode);
> > +
> > +  rtx pat = GEN_FCN (icode) (target);
> > +  if (!pat)
> > +    return NULL_RTX;
> > +  emit_insn (pat);
> 
> I think you need to verify whether the expansion ended up in 'target'
> and otherwise emit a move since usually 'target' is just a hint.

I thought the "if (target == 0 ..." took care of that. The expands do
emit a move, if that helps.

For feclearexcept and feraiseexcept I included tests to variable
'target', including none, but now I see that I did not do the same for
fegetround, I can add the same if it is necessary, but the test do check
if the return is correct, so I don't know.


> > +@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} function from the ISO C99 standard.
> 
> I think this needs to elaborate on the format of the "rounding mode".
> 
> AFAICS you do nothing to marshall with the actually used libc
> implementation which AFAIU can choose arbitrary values for
> the FE_* macros.  I'm not sure we require the compiler to be
> configured for one specific C library and for example require
> matching FE_* macro definitions for all uses of the built
> compiler.
> 
> For the patch at hand you seem to assume the libc "format"
> matches the hardware one (which would of course be reasonable).
> 
> Does that actually hold up when looking at libcs other than 
> glibc supporting powerpc?

I checked in some other libc implementations that have POWER support and
all have the same value as glic for the four rounding modes and the five
exception flags from libc. The libcs implementations I checked are:

 - musl
 - uclibc & uclibc-ng
 - freebsd

Is There any other I am missing?


> If all of these are non-issues then the middle-end pices look OK.
> If we need any such "translation" layer then I guess we need
> to either have additional operands to the optabs specifying
> all of the FE_* values relevant for the respective call or
> provide a side-channel (target hook) to implement the
> translation on the expansion side.

IMHO, It seems like it is not necessary if there not a libc that have
different values for the FE_* macros. I didn't check other archs, but if
is the case for some other arch I think it could be changed if and when
some other arch implements expands for these builtins.


o/
Raoni

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Joseph Myers]

On Wed, 4 Nov 2020, Raoni Fassina Firmino via Gcc-patches wrote:

> IMHO, It seems like it is not necessary if there not a libc that have
> different values for the FE_* macros. I didn't check other archs, but if
> is the case for some other arch I think it could be changed if and when
> some other arch implements expands for these builtins.

SPARC is the case I know of where the FE_* values vary depending on target 
libc (see the SPARC_LOW_FE_EXCEPT_VALUES target macro).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Joseph Myers]

On Wed, 4 Nov 2020, Richard Biener wrote:

> AFAICS you do nothing to marshall with the actually used libc
> implementation which AFAIU can choose arbitrary values for
> the FE_* macros.  I'm not sure we require the compiler to be
> configured for one specific C library and for example require
> matching FE_* macro definitions for all uses of the built
> compiler.

The compiler is definitely expected to match a given C library.  This 
applies for <stdint.h> and other typedefs, for example (various of which 
are used for printf format checking).  It also applies to FE_* in some 
cases where relevant for __atomic_feraiseexcept for floating-point atomic 
compound assignment.

> Now, I wonder whether _GCC_ should provide the FE_* macros, thus
> move (parts of) fenv.h to GCC like we do for stdint.h?

I think that would be a bad idea.  fenv.h involves library functionality 
that can sometimes need to do things beyond simply modifying hardware 
registers.  Consider e.g. the TLS exception and rounding mode state for 
soft-float powerpc-linux-gnu.  Or the TLS decimal rounding mode in libdfp.  
Or how exception enabling can involve a prctl call on powerpc.  Getting 
libgcc involved in storing such TLS state seems problematic.  And whether 
an FE_* macro should be defined may depend on whether the library supports 
the underlying functionality (consider the case of FE_TONEARESTFROMZERO 
for RISC-V, where defining it should mean library code actually works in 
that rounding mode, not just that hardware supports it).

The natural way to handle the rule in C2x that "The strictly conforming 
programs that shall be accepted by a conforming freestanding 
implementation that defines __STDC_IEC_60559_BFP__ or 
__STDC_IEC_60559_DFP__ may also use features in the contents of the 
standard headers <fenv.h> and <math.h> and the numeric conversion 
functions (7.22.1) of the standard header <stdlib.h>." would be to say 
that GCC provides the compiler pieces of a freestanding implementation, 
not necessarily the whole freestanding implementation.  (Those macros 
would only be defined via implicit preinclusion of stdc-predef.h anyway.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Jeff Law]

On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
> On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
>>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
>>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
>>> +static rtx
>>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
>>> +{
>>> +  if (!validate_arglist (exp, VOID_TYPE))
>>> +    return NULL_RTX;
>>> +
>>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
>>> +  if (icode == CODE_FOR_nothing)
>>> +    return NULL_RTX;
>>> +
>>> +  if (target == 0
>>> +      || GET_MODE (target) != target_mode
>>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
>>> +    target = gen_reg_rtx (target_mode);
>>> +
>>> +  rtx pat = GEN_FCN (icode) (target);
>>> +  if (!pat)
>>> +    return NULL_RTX;
>>> +  emit_insn (pat);
>> I think you need to verify whether the expansion ended up in 'target'
>> and otherwise emit a move since usually 'target' is just a hint.
> I thought the "if (target == 0 ..." took care of that. The expands do
> emit a move, if that helps.
It looks like if we have a passed in target and it either has the wrong
mode or it does not match the predicate, then we generaet a new target
and use that instead.  I don't see where we'd copy from that new target
to the original desired target.  For some expanders the caller would
handle that, but I don't see how that's possible for this one without
the caller digging into the generated RTL to determine that
expand_builtin_fegetround put the result somewhere other than TARGET and
thus a copy is needed.

That may be what Richi is worried about.

Jeff

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Jeff Law]

On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
> On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
>>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
>>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
>>> +static rtx
>>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
>>> +{
>>> +  if (!validate_arglist (exp, VOID_TYPE))
>>> +    return NULL_RTX;
>>> +
>>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
>>> +  if (icode == CODE_FOR_nothing)
>>> +    return NULL_RTX;
>>> +
>>> +  if (target == 0
>>> +      || GET_MODE (target) != target_mode
>>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
>>> +    target = gen_reg_rtx (target_mode);
>>> +
>>> +  rtx pat = GEN_FCN (icode) (target);
>>> +  if (!pat)
>>> +    return NULL_RTX;
>>> +  emit_insn (pat);
>> I think you need to verify whether the expansion ended up in 'target'
>> and otherwise emit a move since usually 'target' is just a hint.
> I thought the "if (target == 0 ..." took care of that. The expands do
> emit a move, if that helps.
>
> For feclearexcept and feraiseexcept I included tests to variable
> 'target', including none, but now I see that I did not do the same for
> fegetround, I can add the same if it is necessary, but the test do check
> if the return is correct, so I don't know.
>
>
>>> +@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} function from the ISO C99 standard.
>> I think this needs to elaborate on the format of the "rounding mode".
>>
>> AFAICS you do nothing to marshall with the actually used libc
>> implementation which AFAIU can choose arbitrary values for
>> the FE_* macros.  I'm not sure we require the compiler to be
>> configured for one specific C library and for example require
>> matching FE_* macro definitions for all uses of the built
>> compiler.
>>
>> For the patch at hand you seem to assume the libc "format"
>> matches the hardware one (which would of course be reasonable).
>>
>> Does that actually hold up when looking at libcs other than 
>> glibc supporting powerpc?
> I checked in some other libc implementations that have POWER support and
> all have the same value as glic for the four rounding modes and the five
> exception flags from libc. The libcs implementations I checked are:
>
>  - musl
>  - uclibc & uclibc-ng
>  - freebsd
>
> Is There any other I am missing?
I think the concern here is that while the libcs we have visibility into
have consistent values, I don't think that's necessarily guaranteed.  
I'm not sure how to DTRT here.  We may not have the target headers if
we're doing a cross compile, so a configure test may not do what we 
need.  In fact, ISTM that there is no reliable configure or compile time
check we can do since the constants are part of the runtime and can
change independently of the compiler.

Jeff

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Richard Biener]

On Tue, 17 Nov 2020, Jeff Law wrote:

> 
> 
> On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
> > On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
> >>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
> >>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
> >>> +static rtx
> >>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
> >>> +{
> >>> +  if (!validate_arglist (exp, VOID_TYPE))
> >>> +    return NULL_RTX;
> >>> +
> >>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
> >>> +  if (icode == CODE_FOR_nothing)
> >>> +    return NULL_RTX;
> >>> +
> >>> +  if (target == 0
> >>> +      || GET_MODE (target) != target_mode
> >>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
> >>> +    target = gen_reg_rtx (target_mode);
> >>> +
> >>> +  rtx pat = GEN_FCN (icode) (target);
> >>> +  if (!pat)
> >>> +    return NULL_RTX;
> >>> +  emit_insn (pat);
> >> I think you need to verify whether the expansion ended up in 'target'
> >> and otherwise emit a move since usually 'target' is just a hint.
> > I thought the "if (target == 0 ..." took care of that. The expands do
> > emit a move, if that helps.
> It looks like if we have a passed in target and it either has the wrong
> mode or it does not match the predicate, then we generaet a new target
> and use that instead.? I don't see where we'd copy from that new target
> to the original desired target.? For some expanders the caller would
> handle that, but I don't see how that's possible for this one without
> the caller digging into the generated RTL to determine that
> expand_builtin_fegetround put the result somewhere other than TARGET and
> thus a copy is needed.
> 
> That may be what Richi is worried about.

I know we've added missing

  if (!rtx_equal_p (target, ops[0].value))
    emit_move_insn (target, ops[0].value);

to several expanders (using expand_insn rather than manual
GEN_FCN (icode) calls).

Richard.

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Segher Boessenkool]

On Wed, Nov 18, 2020 at 08:31:28AM +0100, Richard Biener wrote:
> On Tue, 17 Nov 2020, Jeff Law wrote:
> > On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
> > > On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
> > >>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
> > >>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
> > >>> +static rtx
> > >>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
> > >>> +{
> > >>> +  if (!validate_arglist (exp, VOID_TYPE))
> > >>> +    return NULL_RTX;
> > >>> +
> > >>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
> > >>> +  if (icode == CODE_FOR_nothing)
> > >>> +    return NULL_RTX;
> > >>> +
> > >>> +  if (target == 0
> > >>> +      || GET_MODE (target) != target_mode
> > >>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
> > >>> +    target = gen_reg_rtx (target_mode);
> > >>> +
> > >>> +  rtx pat = GEN_FCN (icode) (target);
> > >>> +  if (!pat)
> > >>> +    return NULL_RTX;
> > >>> +  emit_insn (pat);
> > >> I think you need to verify whether the expansion ended up in 'target'
> > >> and otherwise emit a move since usually 'target' is just a hint.
> > > I thought the "if (target == 0 ..." took care of that. The expands do
> > > emit a move, if that helps.
> > It looks like if we have a passed in target and it either has the wrong
> > mode or it does not match the predicate, then we generaet a new target
> > and use that instead.? I don't see where we'd copy from that new target
> > to the original desired target.? For some expanders the caller would
> > handle that, but I don't see how that's possible for this one without
> > the caller digging into the generated RTL to determine that
> > expand_builtin_fegetround put the result somewhere other than TARGET and
> > thus a copy is needed.
> > 
> > That may be what Richi is worried about.
> 
> I know we've added missing
> 
>   if (!rtx_equal_p (target, ops[0].value))
>     emit_move_insn (target, ops[0].value);
> 
> to several expanders (using expand_insn rather than manual
> GEN_FCN (icode) calls).

We can handle the constants issue similarly to what we do for
__builtin_fpclassify, too.


Segher

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Jeff Law]

On 11/18/20 12:31 AM, Richard Biener wrote:
> On Tue, 17 Nov 2020, Jeff Law wrote:
>
>>
>> On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
>>> On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
>>>>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
>>>>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
>>>>> +static rtx
>>>>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
>>>>> +{
>>>>> +  if (!validate_arglist (exp, VOID_TYPE))
>>>>> +    return NULL_RTX;
>>>>> +
>>>>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
>>>>> +  if (icode == CODE_FOR_nothing)
>>>>> +    return NULL_RTX;
>>>>> +
>>>>> +  if (target == 0
>>>>> +      || GET_MODE (target) != target_mode
>>>>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
>>>>> +    target = gen_reg_rtx (target_mode);
>>>>> +
>>>>> +  rtx pat = GEN_FCN (icode) (target);
>>>>> +  if (!pat)
>>>>> +    return NULL_RTX;
>>>>> +  emit_insn (pat);
>>>> I think you need to verify whether the expansion ended up in 'target'
>>>> and otherwise emit a move since usually 'target' is just a hint.
>>> I thought the "if (target == 0 ..." took care of that. The expands do
>>> emit a move, if that helps.
>> It looks like if we have a passed in target and it either has the wrong
>> mode or it does not match the predicate, then we generaet a new target
>> and use that instead.? I don't see where we'd copy from that new target
>> to the original desired target.? For some expanders the caller would
>> handle that, but I don't see how that's possible for this one without
>> the caller digging into the generated RTL to determine that
>> expand_builtin_fegetround put the result somewhere other than TARGET and
>> thus a copy is needed.
>>
>> That may be what Richi is worried about.
> I know we've added missing
>
>   if (!rtx_equal_p (target, ops[0].value))
>     emit_move_insn (target, ops[0].value);
>
> to several expanders (using expand_insn rather than manual
> GEN_FCN (icode) calls).
Yes.  But I think we end up doing that mostly for expanders that return
the object where the value was stored in some reasonably convenient
location (either as a return value or in an ops array).  I don't think
that's the case here. 

Jeff

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Raoni Fassina Firmino]

It seems to me we have two unrelated concerns mixed in the threads, I
will reply in two different sub-threads to make this easier.

This one to discuss the handling of target and output from the expands


On Wed, Nov 18, 2020 at 02:45:44PM -0700, AL gcc-patches wrote:
> 
> 
> On 11/18/20 12:31 AM, Richard Biener wrote:
> > On Tue, 17 Nov 2020, Jeff Law wrote:
> >
> >>
> >> On 11/4/20 8:10 AM, Raoni Fassina Firmino via Gcc-patches wrote:
> >>> On Wed, Nov 04, 2020 at 10:35:03AM +0100, Richard Biener wrote:
> >>>>> +/* Expand call EXP to the fegetround builtin (from C99 fenv.h), returning the
> >>>>> +   result and setting it in TARGET.  Otherwise return NULL_RTX on failure.  */
> >>>>> +static rtx
> >>>>> +expand_builtin_fegetround (tree exp, rtx target, machine_mode target_mode)
> >>>>> +{
> >>>>> +  if (!validate_arglist (exp, VOID_TYPE))
> >>>>> +    return NULL_RTX;
> >>>>> +
> >>>>> +  insn_code icode = direct_optab_handler (fegetround_optab, SImode);
> >>>>> +  if (icode == CODE_FOR_nothing)
> >>>>> +    return NULL_RTX;
> >>>>> +
> >>>>> +  if (target == 0
> >>>>> +      || GET_MODE (target) != target_mode
> >>>>> +      || !(*insn_data[icode].operand[0].predicate) (target, target_mode))
> >>>>> +    target = gen_reg_rtx (target_mode);
> >>>>> +
> >>>>> +  rtx pat = GEN_FCN (icode) (target);
> >>>>> +  if (!pat)
> >>>>> +    return NULL_RTX;
> >>>>> +  emit_insn (pat);
> >>>> I think you need to verify whether the expansion ended up in 'target'
> >>>> and otherwise emit a move since usually 'target' is just a hint.
> >>> I thought the "if (target == 0 ..." took care of that. The expands do
> >>> emit a move, if that helps.
> >> It looks like if we have a passed in target and it either has the wrong
> >> mode or it does not match the predicate, then we generaet a new target
> >> and use that instead.? I don't see where we'd copy from that new target
> >> to the original desired target.? For some expanders the caller would
> >> handle that, but I don't see how that's possible for this one without
> >> the caller digging into the generated RTL to determine that
> >> expand_builtin_fegetround put the result somewhere other than TARGET and
> >> thus a copy is needed.
> >>
> >> That may be what Richi is worried about.
> > I know we've added missing
> >
> >   if (!rtx_equal_p (target, ops[0].value))
> >     emit_move_insn (target, ops[0].value);
> >
> > to several expanders (using expand_insn rather than manual
> > GEN_FCN (icode) calls).
> Yes.  But I think we end up doing that mostly for expanders that return
> the object where the value was stored in some reasonably convenient
> location (either as a return value or in an ops array).  I don't think
> that's the case here. 

So, I think I got it wrong then, I thought the semantics where that
the expander was responsible to provide a suitable target to the
expand, and the expand was responsible to output to that target.  That
is how I created both, so if the expand can change the target maybe
then it should be also responsible to generate the correct target.
But this seems to me that we will have more repeated code for expands
in different archs.


o/
Raoni Fassina

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Raoni Fassina Firmino]

It seems to me we have two unrelated concerns mixed in the threads, I
will reply in two different sub-threads to make this easier.

This one to discuss the values of FE_* macros.


On Tue, Nov 17, 2020 at 03:23:02PM -0700, AL gcc-patches wrote:
> >>> +@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} function from the ISO C99 standard.
> >> I think this needs to elaborate on the format of the "rounding mode".
> >>
> >> AFAICS you do nothing to marshall with the actually used libc
> >> implementation which AFAIU can choose arbitrary values for
> >> the FE_* macros.  I'm not sure we require the compiler to be
> >> configured for one specific C library and for example require
> >> matching FE_* macro definitions for all uses of the built
> >> compiler.
> >>
> >> For the patch at hand you seem to assume the libc "format"
> >> matches the hardware one (which would of course be reasonable).
> >>
> >> Does that actually hold up when looking at libcs other than 
> >> glibc supporting powerpc?
> > I checked in some other libc implementations that have POWER support and
> > all have the same value as glic for the four rounding modes and the five
> > exception flags from libc. The libcs implementations I checked are:
> >
> >  - musl
> >  - uclibc & uclibc-ng
> >  - freebsd
> >
> > Is There any other I am missing?
> I think the concern here is that while the libcs we have visibility into
> have consistent values, I don't think that's necessarily guaranteed.  
> I'm not sure how to DTRT here.  We may not have the target headers if
> we're doing a cross compile, so a configure test may not do what we 
> need.  In fact, ISTM that there is no reliable configure or compile time
> check we can do since the constants are part of the runtime and can
> change independently of the compiler.


From other subthreads Joseph and segher mentioned this:

On Wed, Nov 04, 2020 at 09:06:02PM +0000, Joseph Myers wrote:
> On Wed, 4 Nov 2020, Raoni Fassina Firmino via Gcc-patches wrote:
> 
> > IMHO, It seems like it is not necessary if there not a libc that have
> > different values for the FE_* macros. I didn't check other archs, but if
> > is the case for some other arch I think it could be changed if and when
> > some other arch implements expands for these builtins.
> 
> SPARC is the case I know of where the FE_* values vary depending on target 
> libc (see the SPARC_LOW_FE_EXCEPT_VALUES target macro).

On Wed, Nov 18, 2020 at 06:38:22AM -0600, Segher Boessenkool wrote:
> We can handle the constants issue similarly to what we do for
> __builtin_fpclassify, too.


I think that if we must safe-guard for future or unforeseen libc
implementations doing what __builtin_fpclassify does is the way to go.
I don't know what is the GCC police here, but IMHO I don't think we
should add complexity before it is needed in this case.  And looking at
__builtin_fpclassify, it seems a lot, IIUC this solution needs
fixinclude to work, seems to me too much add maintenance for something
that is not needed yet, because SPARC don't have this expands, none has
for now.

I don't know if it helps, but the included tests also check the values
changes against the libc implementations, so may catch discrepancies if
building gcc with other libcs.  It, of course, doesn't help if using for
example a gcc built with glibc and compiling a program with it with some
unknown libc.  I wonder if some safe check that can be done at runtime,
whilst building a program with gcc and some unknown libc.


o/
Raoni Fassina

Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Segher Boessenkool]

On Thu, Jan 07, 2021 at 11:20:39AM -0300, Raoni Fassina Firmino wrote:
> On Wed, Nov 18, 2020 at 06:38:22AM -0600, Segher Boessenkool wrote:
> > We can handle the constants issue similarly to what we do for
> > __builtin_fpclassify, too.
> 
> I think that if we must safe-guard for future or unforeseen libc
> implementations doing what __builtin_fpclassify does is the way to go.
> I don't know what is the GCC police here, but IMHO I don't think we
> should add complexity before it is needed in this case.  And looking at
> __builtin_fpclassify, it seems a lot, IIUC this solution needs
> fixinclude to work, seems to me too much add maintenance for something
> that is not needed yet, because SPARC don't have this expands, none has
> for now.

This way the compiler does not need to know the values of the macros
*at all*, that is the whole point!  You simply pass all the standard
values to the builtin as extra arguments.  This may seem inconvenient
to use, but you put the whole thing in a header file anyway, all is
hidden.


Segher