Re: [PATCH] Support Cell SPU floating point

[Geert Bosch <bosch at adacore dot com>]

On Dec 17, 2007, at 18:07, Trevor_Smigiel@playstation.sony.com wrote:
> Ping?
>
> * Trevor Smigiel <Trevor_Smigiel@playstation.sony.com> [2007-10-25  
> 12:15]:
> > Are there any further comments on this patch?
> >
> > If not, is it ok for mainline?
While I can't approve your patch, I have a few comments/questions in the
hope that this will solicit more authoritative review.
> > > ! @defmac DENORM_OPERANDS_ARE_ZERO (@var{mode})
> > > ! If defined, this macro should be true for @var{mode} if a  
> > > denormalized
> > > ! number should be converted to zero before constant folding an  
> > > operation.
> > > !
> > > ! The default definition of this macro is false for all modes.
> > > ! @end defmac
> > > !
> > > ! @defmac DENORM_RESULTS_ARE_ZERO (@var{mode})
> > > ! If defined, this macro should be true for @var{mode} if the  
> > > results of
> > > ! a constant folded operation is a denormalized number which  
> > > should be
> > > ! converted to zero.
> > > !
> > > ! The default definition of this macro is false for all modes.
> > > ! @end defmac
[...]
> > > + /* True when the given mode fully supports denormalized numbers.  
> > > */
> > > + #define HONOR_NONIEEE_DENORMS(MODE) \
> > > +   (!DENORM_OPERANDS_ARE_ZERO (MODE) && !DENORM_RESULTS_ARE_ZERO  
> > > (MODE))
> > > +
So, HONOR_NONIEEE_DENORMS(MODE) is true by default?
This predicate seems dubious, because the definition would apply
to modes that fully honor IEEE denorm semantics.

> > > Index: gcc/builtins.c
> > > ===================================================================
> > > *** gcc/builtins.c	(revision 128153)
> > > --- gcc/builtins.c	(working copy)
> > > *************** fold_builtin_abs (tree arg, tree type)
> > > *** 9291,9297 ****
> > >  static tree
> > >  fold_builtin_fmin_fmax (tree arg0, tree arg1, tree type, bool max)
> > >  {
> > > !   if (validate_arg (arg0, REAL_TYPE) && validate_arg (arg1,  
> > > REAL_TYPE))
> > >      {
> > >        /* Calculate the result when the argument is a constant.  */
> > >        tree res = do_mpfr_arg2 (arg0, arg1, type, (max ?  
> > > mpfr_max : mpfr_min));
> > > --- 9291,9298 ----
> > >  static tree
> > >  fold_builtin_fmin_fmax (tree arg0, tree arg1, tree type, bool max)
> > >  {
> > > !   if (validate_arg (arg0, REAL_TYPE) && validate_arg (arg1,  
> > > REAL_TYPE)
> > > !       && !HONOR_NONIEEE_DENORMS (TYPE_MODE (type)))
> > >      {
> > >        /* Calculate the result when the argument is a constant.  */
> > >        tree res = do_mpfr_arg2 (arg0, arg1, type, (max ?  
> > > mpfr_max : mpfr_min));
It seems that you disable this optimization for modes with regular  
IEEE denorms.
Am I missing something obvious?

Apart from the HONOR_NONIEEE_DENORMS related issues, the  
real_arithmetic_fold and
real_compare_fold seem to make sense: by default we keep the current  
semantics,
which is either dynamic rounding (changeable at run-time) or round-to- 
even,
but separate routines exist taking a mode. We can expand the non-IEEE  
modes we
recognize over time and/or grow the number of functions that support  
such specified
modes.

The name seems strange though, because the difference between  
real_arithmetic_fold
and real_arithmetic is not that one folds and the other doesn't, but  
that one uses
a specific mode and the other doesn't. So a name like  
real_arithmetic_mode or
real_arithmetic_in_mode would seem more descriptive.

  -Geert