Re: [Patch 3/11] Implement TARGET_C_EXCESS_PRECISION for s390

[Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>]

On 09/30/2016 07:57 PM, Joseph Myers wrote:
> On Fri, 30 Sep 2016, Jeff Law wrote:
> 
>> On 09/30/2016 11:34 AM, Joseph Myers wrote:
>>> On Fri, 30 Sep 2016, James Greenhalgh wrote:
>>>
>>>> +      case EXCESS_PRECISION_TYPE_STANDARD:
>>>> +      case EXCESS_PRECISION_TYPE_IMPLICIT:
>>>> +	/* Otherwise, the excess precision we want when we are
>>>> +	   in a standards compliant mode, and the implicit precision we
>>>> +	   provide can be identical.  */
>>>> +	return FLT_EVAL_METHOD_PROMOTE_TO_DOUBLE;
>>>
>>> That's wrong for EXCESS_PRECISION_TYPE_IMPLICIT.  There is no implicit
>>> promotion in the back end (and really there shouldn't be any excess
>>> precision here at all, and double_t in glibc should be fixed along with a
>>> GCC change to remove this mistake).
>> Sorry, change to a NAK.
>>
>> Joseph, what's the right thing to do here?
> 
> (a) The present patch would keep the existing value of FLT_EVAL_METHOD.  
> But the existing value is inaccurate for the default compilation mode, 
> when there is no implicit promotion in the back end, and doing so means 
> suboptimal code in libgcc and glibc because it does things to handle 
> excess precision that isn't actually there (and quite possibly in code 
> elsewhere that looks at FLT_EVAL_METHOD).
> 
> (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

> (c) Removing all special excess precision for S/390 from GCC, and changing 
> float_t to float in glibc, is logically correct and produces optimal code.  
> float_t does not appear in the ABI of any glibc function; in principle it 
> could affect the ABIs of other libraries, but I don't think that's 
> particularly likely.

I really would like to do this.  The idea came up several times already but we always were concerned
about the potential ABI break.

> The only argument for (a) is that's it's semantics-preserving - it's just 
> that the preserved semantics are nonsensical and involve an inaccurate 
> value of FLT_EVAL_METHOD in the default compilation mode.

I'll try to set up some scans on src packages to get a better feel about where it would potentially
break. I'll come back with the results. I do not want to block the patchset with this though. So if
you would like to go on quickly feel free to commit (a).

-Andreas-