Re: soft-fp: Support rsigned == 2 in _FP_TO_INT

["Carlos O'Donell" <carlos at redhat dot com>]

On 09/23/2014 04:41 PM, Joseph S. Myers wrote:
> Continuing the addition of soft-fp features in the Linux kernel
> version, this patch adds _FP_TO_INT support for rsigned == 2 (reduce
> overflowing results modulo 2^rsize to fit in the destination, used for
> alpha emulation).  (This patch is relative to a tree with
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00411.html>,
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00442.html>,
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00461.html>,
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00463.html>,
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00464.html> and
> <https://sourceware.org/ml/libc-alpha/2014-09/msg00488.html> applied;
> there are actual dependencies on at least the fourth and fifth of
> those patches.)
> 
> The kernel version is buggy; it can left shift by a negative amount
> when right shifting is required in an overflow case (the kernel
> version also has other bugs fixed long ago in glibc; at least,
> spurious exceptions converting to the most negative integer).  This
> version avoids that by handling overflow (other than to 0) for rsigned
> == 2 along with the normal non-overflow case, which already properly
> determines the direction in which to shift.
> 
> Tested for powerpc-nofpu.  Some functions get slightly bigger and some
> get slightly smaller, no doubt as a result of the change to where in
> the macro "inexact" is raised, but I don't think those changes are
> significant.  Also tested for powerpc-nofpu with the relevant __fix*
> functions changed to use rsigned == 2 (which is after all just as
> valid as rsigned == 1 in IEEE terms), including verifying the results
> and exceptions for various cases of conversions.
> 
> With these seven patches, the one remaining feature to add for the
> soft-fp code to have all the features of the kernel version is
> _FP_TO_INT_ROUND.
> 
> 2014-09-23  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* soft-fp/op-common.h (_FP_TO_INT): Handle rsigned == 2.

Looks good to me.

> diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h
> index 3deb9b1..cab6fba 100644
> --- a/soft-fp/op-common.h
> +++ b/soft-fp/op-common.h
> @@ -1390,6 +1390,8 @@
>     1:  the number is required to be -(2^(rsize-1))..(2^(rsize-1))-1, if not,
>         NV is set plus the result is either -(2^(rsize-1)) or (2^(rsize-1))-1
>         depending on the sign in such case.
> +   2:  the number is required to be -(2^(rsize-1))..(2^(rsize-1))-1, if not,
> +       NV is set plus the result is reduced modulo 2^rsize.

OK.

>     -1: the number is required to be -(2^(rsize-1))..(2^rsize)-1, if not, NV is
>         set plus the result is either -(2^(rsize-1)) or (2^(rsize-1))-1
>         depending on the sign in such case.  */
> @@ -1411,10 +1413,28 @@
>  	  else								\
>  	    FP_SET_EXCEPTION (FP_EX_INEXACT);				\
>  	}								\
> -      else if (X##_e >= (_FP_EXPMAX_##fs < _FP_EXPBIAS_##fs + rsize	\
> -			 ? _FP_EXPMAX_##fs				\
> -			 : _FP_EXPBIAS_##fs + rsize - (rsigned > 0 || X##_s)) \
> -	       || (!rsigned && X##_s))					\
> +      else if (rsigned == 2						\
> +	       && (X##_e						\
> +		   >= ((_FP_EXPMAX_##fs					\
> +			< _FP_EXPBIAS_##fs + _FP_FRACBITS_##fs + rsize - 1) \
> +		       ? _FP_EXPMAX_##fs				\
> +		       : _FP_EXPBIAS_##fs + _FP_FRACBITS_##fs + rsize - 1))) \
> +	{								\
> +	  /* Overflow resulting in 0.  */				\
> +	  r = 0;							\
> +	  FP_SET_EXCEPTION (FP_EX_INVALID				\
> +			    | FP_EX_INVALID_CVI				\
> +			    | ((FP_EX_INVALID_SNAN			\
> +				&& _FP_ISSIGNAN (fs, wc, X))		\
> +			       ? FP_EX_INVALID_SNAN			\

OK.

> +			       : 0));					\
> +	}								\
> +      else if (rsigned != 2						\
> +	       && (X##_e >= (_FP_EXPMAX_##fs < _FP_EXPBIAS_##fs + rsize	\
> +			     ? _FP_EXPMAX_##fs				\
> +			     : (_FP_EXPBIAS_##fs + rsize		\
> +				- (rsigned > 0 || X##_s)))		\
> +		   || (!rsigned && X##_s)))				\

OK.



>  	{								\
>  	  /* Overflow or converting to the most negative integer.  */	\
>  	  if (rsigned)							\
> @@ -1461,6 +1481,7 @@
>  	}								\
>        else								\
>  	{								\
> +	  int _FP_TO_INT_inexact = 0;					\
>  	  _FP_FRAC_HIGH_RAW_##fs (X) |= _FP_IMPLBIT_##fs;		\
>  	  if (X##_e >= _FP_EXPBIAS_##fs + _FP_FRACBITS_##fs - 1)	\
>  	    {								\
> @@ -1469,17 +1490,27 @@
>  	    }								\
>  	  else								\
>  	    {								\
> -	      int _FP_TO_INT_inexact;					\
>  	      _FP_FRAC_SRST_##wc (X, _FP_TO_INT_inexact,		\
>  				  (_FP_FRACBITS_##fs + _FP_EXPBIAS_##fs - 1 \
>  				   - X##_e),				\
>  				  _FP_FRACBITS_##fs);			\
> -	      if (_FP_TO_INT_inexact)					\
> -		FP_SET_EXCEPTION (FP_EX_INEXACT);			\
>  	      _FP_FRAC_ASSEMBLE_##wc (r, X, rsize);			\
>  	    }								\
>  	  if (rsigned && X##_s)						\
>  	    r = -r;							\
> +	  if (rsigned == 2 && X##_e >= _FP_EXPBIAS_##fs + rsize - 1)	\
> +	    {								\
> +	      /* Overflow or converting to the most negative integer.  */ \
> +	      if (X##_e > _FP_EXPBIAS_##fs + rsize - 1			\
> +		  || !X##_s						\
> +		  || r != (((typeof (r)) 1) << (rsize - 1)))		\
> +		{							\
> +		  _FP_TO_INT_inexact = 0;				\
> +		  FP_SET_EXCEPTION (FP_EX_INVALID | FP_EX_INVALID_CVI);	\
> +		}							\
> +	    }								\
> +	  if (_FP_TO_INT_inexact)					\
> +	    FP_SET_EXCEPTION (FP_EX_INEXACT);				\

OK.

>  	}								\
>      }									\
>    while (0)
> 

Cheers,
Carlos.