Re: RISC-V glibc port, v7

[Andrew Waterman <andrew at sifive dot com>]

On Mon, Jan 29, 2018 at 10:26 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 29 Jan 2018 07:52:35 PST (-0800), joseph@codesourcery.com wrote:
>>
>> Please commit with the changes requested in review of patches 5 and 6
>> (note the latter applies to more patches than just patch 6) - and no other
>> changes.  Do not send a v8 patch series.  Any further changes should be
>> sent separately once the initial port has been committed, and are subject
>> to the usual subsystem maintainer rules.
>
>
> OK, makes sense.
>
>> Once you've committed v7 with the fixes noted, here are some further
>> questions that may require followup patches:
>>
>> * An earlier set of test results posted had strfromf failures, which then
>> disappeared in a later set of test results without any strfromf-related
>> patches that I could see.  My understanding was that those failures
>> probably resulted from the RISC-V rule that most floating-point
>> instructions with NaN results produce a canonical NaN with positive sign -
>> thus, where strfromf converts a float argument to double (in order to use
>> printf code that doesn't handle float arguments) it loses the sign of the
>> original float NaN.  Did I misunderstand this rule in the RISC-V ISA
>> manual?  I was expecting that you'd need e.g. a hook for the strfromf
>> conversion from float to double (inline function in a new sysdeps header)
>> where the generic version just does a trivial conversion but the RISC-V
>> version handles NaNs specially to preserve the sign.
>
>
> We fixed a handful of NaN-related issues in both GCC and QEMU, so it's possible one of these made the failures go away (and possible erroneously).  I'll take a closer look.
>
>> * You're using fmax and fmin instructions to implement the corresponding
>> libm functions.  As far as I can see, this would be correct by the
>> semantics of those instructions in the v2.2 ISA manual - but the current
>> git version of the ISA manual changes the instruction semantics so that
>> they would no longer be correct for those functions (and would instead be
>> correct for fmaximum_num and fminimum_num, in the proposed changes to TS
>> 18661-1 to support IEEE 754-2018, but we don't have those functions in
>> glibc).  Now, if hardware may implement either v2.2 or v2.3 semantics,
>> that makes those instructions fairly useless for glibc, in the absence of
>> a way to mark a glibc binary as requiring a particular ISA version; if
>> hardware implements only v2.3, the instructions are still inappropriate
>> for fmax/fmin/fmaxf/fminf (but could be used in future for fmaximum_num
>> and fminimum_num, if and when those are added to C2x and to glibc).
>
>
> I think this is a question for Andrew.  I'd added him to the email in case he
> wasn't looking.


My reading of Annex F is that either the old or new semantics for
these instructions suffices to implement the C functions, since the
standard generally leaves signaling-NaN behavior implementation
defined.

I see that glibc's generic versions return a quiet NaN and raise the
invalid exception on signaling-NaN inputs.  Annex F's sample code for
fmax does not necessarily return a quiet NaN: { return
(isgreaterequal(x, y) || isnan(y)) ? x : y; }

Is glibc's definition for these functions stricter than required by
the standard, or am I missing something?