Re: [Qemu-devel] [PATCH v2 16/20] fpu/softfloat: re-factor float to int/uint

[Peter Maydell]

On 9 January 2018 at 12:22, Alex Bennée <address@hidden> wrote:
> We share the common int64/uint64_pack_decomposed function across all
> the helpers and simply limit the final result depending on the final
> size.
>
> Signed-off-by: Alex Bennée <address@hidden>
>
> --
> v2
>   - apply float_flg_invalid fixes next patch
> ---
>  fpu/softfloat.c         | 1011 
> +++++++++++------------------------------------
>  include/fpu/softfloat.h |   13 +
>  2 files changed, 235 insertions(+), 789 deletions(-)
>

> -/*----------------------------------------------------------------------------
> -| Returns the result of converting the double-precision floating-point value
> -| `a' to the 64-bit two's complement integer format.  The conversion is
> -| performed according to the IEC/IEEE Standard for Binary Floating-Point
> -| Arithmetic---which means in particular that the conversion is rounded
> -| according to the current rounding mode.  If `a' is a NaN, the largest
> -| positive integer is returned.  Otherwise, if the conversion overflows, the
> -| largest integer with the same sign as `a' is returned.
> +| Standard for Binary Floating-Point Arithmetic.
>  
> *----------------------------------------------------------------------------*/
>
> -int64_t float64_to_int64(float64 a, float_status *status)
> +int float32_lt_quiet(float32 a, float32 b, float_status *status)
>  {
> -    flag aSign;
> -    int aExp;
> -    int shiftCount;
> -    uint64_t aSig, aSigExtra;
> -    a = float64_squash_input_denormal(a, status);
> +    flag aSign, bSign;
> +    uint32_t av, bv;
> +    a = float32_squash_input_denormal(a, status);
> +    b = float32_squash_input_denormal(b, status);
>
> -    aSig = extractFloat64Frac( a );
> -    aExp = extractFloat64Exp( a );
> -    aSign = extractFloat64Sign( a );
> -    if ( aExp ) aSig |= LIT64( 0x0010000000000000 );
> -    shiftCount = 0x433 - aExp;
> -    if ( shiftCount <= 0 ) {
> -        if ( 0x43E < aExp ) {
> +    if (    ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a ) )
> +         || ( ( extractFloat32Exp( b ) == 0xFF ) && extractFloat32Frac( b ) )
> +       ) {
> +        if (float32_is_signaling_nan(a, status)
> +         || float32_is_signaling_nan(b, status)) {


Is this actually you changing existing code, or is it just that
diff has got confused? If the latter, perhaps whatever the
"think a bit harder" flag to diff is might make the patch
easier to read?

thanks
-- PMM