Re: [Qemu-arm] [PATCH v9 01/10] softfloat: Implement run-time-configurable meaning of signaling NaN bit

[Leon Alrae]

On Fri, Jun 10, 2016 at 11:57:28AM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <address@hidden>
> 
> This patch modifies SoftFloat library so that it can be configured in
> run-time in relation to the meaning of signaling NaN bit, while, at the
> same time, strictly preserving its behavior on all existing platforms.
> 
> Background:
> 
> In floating-point calculations, there is a need for denoting undefined or
> unrepresentable values. This is achieved by defining certain floating-point
> numerical values to be NaNs (which stands for "not a number"). For additional
> reasons, virtually all modern floating-point unit implementations use two
> kinds of NaNs: quiet and signaling. The binary representations of these two
> kinds of NaNs, as a rule, differ only in one bit (that bit is, traditionally,
> the first bit of mantissa).
> 
> Up to 2008, standards for floating-point did not specify all details about
> binary representation of NaNs. More specifically, the meaning of the bit
> that is used for distinguishing between signaling and quiet NaNs was not
> strictly prescribed. (IEEE 754-2008 was the first floating-point standard
> that defined that meaning clearly, see [1], p. 35) As a result, different
> platforms took different approaches, and that presented considerable
> challenge for multi-platform emulators like QEMU.
> 
> Mips platform represents the most complex case among QEMU-supported
> platforms regarding signaling NaN bit. Up to the Release 6 of Mips
> architecture, "1" in signaling NaN bit denoted signaling NaN, which is
> opposite to IEEE 754-2008 standard. From Release 6 on, Mips architecture
> adopted IEEE standard prescription, and "0" denotes signaling NaN. On top of
> that, Mips architecture for SIMD (also known as MSA, or vector instructions)
> also specifies signaling bit in accordance to IEEE standard. MSA unit can be
> implemented with both pre-Release 6 and Release 6 main processor units.
> 
> QEMU uses SoftFloat library to implement various floating-point-related
> instructions on all platforms. The current QEMU implementation allows for
> defining meaning of signaling NaN bit during build time, and is implemented
> via preprocessor macro called SNAN_BIT_IS_ONE.
> 
> On the other hand, the change in this patch enables SoftFloat library to be
> configured in run-time. This configuration is meant to occur during CPU
> initialization, at the moment when it is definitely known what desired
> behavior for particular CPU (or any additional FPUs) is.
> 
> The change is implemented so that it is consistent with existing
> implementation of similar cases. This means that structure float_status is
> used for passing the information about desired signaling NaN bit on each
> invocation of SoftFloat functions. The additional field in float_status is
> called snan_bit_is_one, which supersedes macro SNAN_BIT_IS_ONE.
> 
> IMPORTANT:
> 
> This change is not meant to create any change in emulator behavior or
> functionality on any platform. It just provides the means for SoftFloat
> library to be used in a more flexible way - in other words, it will just
> prepare SoftFloat library for usage related to Mips platform and its
> specifics regarding signaling bit meaning, which is done in some of
> subsequent patches from this series.
> 
> Further break down of changes:
> 
>   1) Added field snan_bit_is_one to the structure float_status, and
>      correspondent setter function set_snan_bit_is_one().
> 
>   2) Constants <float16|float32|float64|floatx80|float128>_default_nan
>      (used both internally and externally) converted to functions
>      <float16|float32|float64|floatx80|float128>_default_nan(float_status*).
>      This is necessary since they are dependent on signaling bit meaning.
>      At the same time, for the sake of code cleanup and simplicity, constants
>      <floatx80|float128>_default_nan_<low|high> (used only internally within
>      SoftFloat library) are removed, as not needed.
> 
>   3) Added a float_status* argument to SoftFloat library functions
>      XXX_is_quiet_nan(XXX a_), XXX_is_signaling_nan(XXX a_),
>      XXX_maybe_silence_nan(XXX a_). This argument must be present in
>      order to enable correct invocation of new version of functions
>      XXX_default_nan(). (XXX is <float16|float32|float64|floatx80|float128>
>      here)
> 
>   4) Updated code for all platforms to reflect changes in SoftFloat library.
>      This change is twofolds: it includes modifications of SoftFloat library
>      functions invocations, and an addition of invocation of function
>      set_snan_bit_is_one() during CPU initialization, with arguments that
>      are appropriate for each particular platform. It was established that
>      all platforms zero their main CPU data structures, so snan_bit_is_one(0)
>      in appropriate places is not added, as it is not needed.
> 
> [1] "IEEE Standard for Floating-Point Arithmetic",
>     IEEE Computer Society, August 29, 2008.
> 
> Tested-by: Bastian Koppelmann <address@hidden> (TriCore part)
> Signed-off-by: Thomas Schwinge <address@hidden>
> Signed-off-by: Maciej W. Rozycki <address@hidden>
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
>  fpu/softfloat-specialize.h    | 535 
> ++++++++++++++++++++----------------------
>  fpu/softfloat.c               | 172 ++++++--------
>  include/fpu/softfloat.h       |  45 ++--
>  target-arm/helper-a64.c       |  14 +-
>  target-arm/helper.c           |  40 ++--
>  target-m68k/helper.c          |   6 +-
>  target-microblaze/op_helper.c |   6 +-
>  target-mips/cpu.h             |   5 +
>  target-mips/helper.h          |   4 +-
>  target-mips/msa_helper.c      |  88 +++----
>  target-mips/op_helper.c       |  17 +-
>  target-mips/translate.c       |   5 +-
>  target-mips/translate_init.c  |   2 +
>  target-ppc/fpu_helper.c       | 120 +++++-----
>  target-s390x/fpu_helper.c     |  28 ++-
>  target-s390x/helper.h         |   6 +-
>  target-s390x/translate.c      |   6 +-
>  target-sh4/cpu.c              |   1 +
>  target-unicore32/cpu.c        |   2 +
>  19 files changed, 547 insertions(+), 555 deletions(-)

Looks good to me (plus I ran my regression tests for MIPS):

Reviewed-by: Leon Alrae <address@hidden>
Tested-by: Leon Alrae <address@hidden>

Thanks,
Leon