On Fri, Oct 07, 2016 at 10:34:25AM +0200, Andreas Krebbel wrote:
> On 10/04/2016 03:42 PM, Joseph Myers wrote:
> > On Tue, 4 Oct 2016, Andreas Krebbel wrote:
> >
> >>> EXCESS_PRECISION_TYPE_FAST would accurately describe what the back end
> >>> does.  It would mean that the default FLT_EVAL_METHOD is 0, which is a
> >>> more accurate description of how the compiler actually behaves, and would
> >>> avoid the suboptimal code in libgcc and glibc.  It would however mean that
> >>> unless -fexcess-precision=standard is used, FLT_EVAL_METHOD (accurate) is
> >>> out of synx with float_t in math.h (inaccurate).
> >>
> >> With (b) we would violate the C standard which explicitly states that
> >> the definition of float_t needs to be float if FLT_EVAL_METHOD is 0.
> >> I've no idea how much code really relies on that. So far I only know
> >> about the Plum Hall testsuite ;) So this probably would still be a safe
> >> change. Actually it was like that for many years without any problems
> >> ... until I've changed it due to the Plum Hall finding :(
> >> https://gcc.gnu.org/ml/gcc-patches/2013-03/msg01124.html
> >
> > You'd only violate it outside standards conformance modes (which you
> > should be using for running conformance testsuites); with -std=c11 etc.
> > -fexcess-precision=standard would be implied, meaning FLT_EVAL_METHOD
> > remains as 1 in that case.
> wrt (b): Agreed. I was more concerned about all the other code which might 
> accidently be built
> without a strict standard compliance option. I did some searches in the 
> source package repos. The
> only snippet where the definition of FLT_EVAL_METHOD might affect code 
> generation is in musl libc
> but that one is being built with -std=c99. So I don't see anything speaking 
> against (b). I'm ok with
> going that way.
> wrt (c): float_t appears to be more widely used than I expected. But the only 
> hits which might
> indicate potential ABI problems where in clucene and libassa. (I've scanned 
> the header files of
> about 25k Ubuntu source packages).
> I'm also not sure about script language interfaces with C. There might be 
> potential problems out
> there which I wasn't able to catch with my scan.
> While I fully agree with you that the float_t type definition for S/390 in 
> Glibc is plain wrong I do
> not really feel comfortable with changing it.
> An interesting case is imagemagick. They define their ABI-relevant 
> MagickRealType based on the size
> of float_t in recent versions but excplicitly without depending on 
> (http://www.imagemagick.org/discourse-server/viewtopic.php?t=22136). They 
> build with -std=gnu99 so
> this helps us with (b) I think. To my understanding MagickRealType would stay 
> double not affected by
> FLT_EVAL_METHOD changes.

Here is a patch implementing what I think has been discussed in this thread.




2016-10-14  James Greenhalgh  <james.greenha...@arm.com>

        * config/s390/s390.c (s390_excess_precision): New.

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index f69b470..8f6f199 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -15107,6 +15107,43 @@ s390_invalid_binary_op (int op ATTRIBUTE_UNUSED, const_tree type1, const_tree ty
   return NULL;
+   FIXME: For historical reasons, float_t and double_t are typedef'ed to
+   double on s390, causing operations on float_t to operate in a higher
+   precision than is necessary.  However, it is not the case that SFmode
+   operations have implicit excess precision, and we generate more optimal
+   code if we let the compiler know no implicit extra precision is added.
+   That means when we are compiling with -fexcess-precision=fast, the value
+   we set for FLT_EVAL_METHOD will be out of line with the actual precision of
+   float_t (though they would be correct for -fexcess-precision=standard).
+   A complete fix would modify glibc to remove the unnecessary typedef
+   of float_t to double.  */
+static enum flt_eval_method
+s390_excess_precision (enum excess_precision_type type)
+  switch (type)
+    {
+	/* The fastest type to promote to will always be the native type,
+	   whether that occurs with implicit excess precision or
+	   otherwise.  */
+	/* Otherwise, when we are in a standards compliant mode, to
+	   ensure consistency with the implementation in glibc, report that
+	   float is evaluated to the range and precision of double.  */
+      default:
+	gcc_unreachable ();
+    }
 /* Initialize GCC target structure.  */
@@ -15167,6 +15204,9 @@ s390_invalid_binary_op (int op ATTRIBUTE_UNUSED, const_tree type1, const_tree ty
 #define TARGET_ASM_CAN_OUTPUT_MI_THUNK hook_bool_const_tree_hwi_hwi_const_tree_true
+#define TARGET_C_EXCESS_PRECISION s390_excess_precision
 #define TARGET_SCHED_ADJUST_PRIORITY s390_adjust_priority

Reply via email to