Thanks again for your feedback.
On Thu, Jan 12, 2017 at 3:30 PM, Joseph Myers <jos...@codesourcery.com> wrote:
> On Wed, 11 Jan 2017, Palmer Dabbelt wrote:
>
>> +riscv*-*-linux*)
>> + tmake_file="${tmake_file} t-softfp-sfdf riscv/t-softfp${host_address}
>> t-softfp riscv/t-elf riscv/t-elf${host_address}"
>> + extra_parts="$extra_parts crtbegin.o crtend.o crti.o crtn.o crtendS.o
>> crtbeginT.o"
>> + md_unwind_header=riscv/linux-unwind.h
>> + ;;
>> +riscv*-*-*)
>> + tmake_file="${tmake_file} t-softfp-sfdf riscv/t-softfp${host_address}
>> t-softfp riscv/t-elf riscv/t-elf${host_address}"
>> + extra_parts="$extra_parts crtbegin.o crtend.o crti.o crtn.o"
>> + ;;
>
> This looks like you're building soft-fp functions into libgcc for all
> types whether or not you have hardware floating point support for them.
>
> If your ABIs are such that hardware and software floating point are ABI
> compatible at the function call level, then both copies of libgcc (the
> shared library, at least) should indeed have the same ABI (so a soft-float
> program can run with a hard-float copy of libgcc) - but for the hardware
> types, it's better to use t-hardfp. If they are not ABI compatible, it's
> best for libgcc to contain only the functions that are actually needed.
> That is, in general, it only needs to contain functions that are not
> implemented in hardware, or are implemented in hardware but might not be
> implemented in hardware for some configurations using the same ABI (and in
> the latter case, the t-hardfp implementations are preferred).
Yes, some soft-float routines are needlessly built for some ABIs.
We'll rectify this.
>
>> +#define _FP_NANFRAC_S ((_FP_QNANBIT_S << 1) - 1)
>> +#define _FP_NANFRAC_D ((_FP_QNANBIT_D << 1) - 1), -1
>> +#define _FP_NANFRAC_Q ((_FP_QNANBIT_Q << 1) - 1), -1, -1, -1
>
> This is different from the default NaN the specification says is used by
> hardware (all mantissa bits clear except for the MSB used to indicate a
> quiet NaN). I'd expect the soft-fp configuration to make the same choices
> here as hardware.
>
>> +#define _FP_NANFRAC_S ((_FP_QNANBIT_S << 1) - 1)
>> +#define _FP_NANFRAC_D ((_FP_QNANBIT_D << 1) - 1)
>> +#define _FP_NANFRAC_Q ((_FP_QNANBIT_Q << 1) - 1), -1
>
> Likewise.
>
>> +#define _FP_KEEPNANFRACP 1
>
> And since the hardware semantics don't propagate payloads I'd expect this
> to be zero, and ...
>
>> +/* From my experiments it seems X is chosen unless one of the
>> + NaNs is sNaN, in which case the result is NANSIGN/NANFRAC. */
>> +#define _FP_CHOOSENAN(fs, wc, R, X, Y, OP) \
>
> ... this to use a canonical NaN unconditionally, so that again you do the
> same as hardware (the comment here is actively misleading in this case as
> it describes something contrary to the hardware specification as being
> what experiments show hardware does).
Thanks for pointing this out. We will make the soft-float canonical
NaN value and NaN propagation behavior match the ISA.
>
>> +#define FP_ROUNDMODE (_fcw >> 5)
>
> I'm unclear from the specification whether the high 24 bits of fcsr are
> architecturally defined always to read as zero, or whether that's only the
> case in present architecture versions and they are reserved for possible
> future feature additions. If the latter, it would seem desirable to mask
> the result of shifting so existing binaries using the soft-fp code
> continue to work on future hardware that might set some of the high bits.
I will see to it that the ISA spec is clarified on this point. In the
mean time, I will obviate the issue by accessing the rounding mode and
exceptions through the frm and fflags shadow CSRs, rather than through
the fcsr. (This should also be more performant.)
>
>> +#define __LITTLE_ENDIAN 1234
>> +#define __BIG_ENDIAN 4321
>> +
>> +#if defined __big_endian__
>> +# define __BYTE_ORDER __BIG_ENDIAN
>> +#else
>> +# define __BYTE_ORDER __LITTLE_ENDIAN
>> +#endif
>
> As far as I can tell the port is always little-endian and there is no
> __big_endian__ macro, so that #if should not be there.
Indeed.
>
> --
> Joseph S. Myers
> jos...@codesourcery.com
