Hi, 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: > > > >>> (b) Handling EXCESS_PRECISION_TYPE_IMPLICIT like > >>> 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 > FLT_EVAL_METHOD > (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. OK? Thanks, James --- gcc/ 2016-10-14 James Greenhalgh <james.greenha...@arm.com> * config/s390/s390.c (s390_excess_precision): New. (TARGET_C_EXCESS_PRECISION): Define.
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; } +/* Implement TARGET_C_EXCESS_PRECISION. + + 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) + { + case EXCESS_PRECISION_TYPE_IMPLICIT: + case EXCESS_PRECISION_TYPE_FAST: + /* The fastest type to promote to will always be the native type, + whether that occurs with implicit excess precision or + otherwise. */ + return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT; + case EXCESS_PRECISION_TYPE_STANDARD: + /* 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. */ + return FLT_EVAL_METHOD_PROMOTE_TO_DOUBLE; + default: + gcc_unreachable (); + } + return FLT_EVAL_METHOD_UNPREDICTABLE; +} + /* Initialize GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP @@ -15167,6 +15204,9 @@ s390_invalid_binary_op (int op ATTRIBUTE_UNUSED, const_tree type1, const_tree ty #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK #define TARGET_ASM_CAN_OUTPUT_MI_THUNK hook_bool_const_tree_hwi_hwi_const_tree_true +#undef TARGET_C_EXCESS_PRECISION +#define TARGET_C_EXCESS_PRECISION s390_excess_precision + #undef TARGET_SCHED_ADJUST_PRIORITY #define TARGET_SCHED_ADJUST_PRIORITY s390_adjust_priority #undef TARGET_SCHED_ISSUE_RATE