Re: [PATCH V2] Aarch32/64: Support __FLT_EVAL_METHOD__ values other than 0, 1, 2

[Jeff Johnston <jjohnstn@redhat.com>]

On Fri, Apr 8, 2022 at 6:09 AM Richard Earnshaw <
Richard.Earnshaw@foss.arm.com> wrote:

>
>
> On 04/04/2022 15:53, Andrea Corallo wrote:
> > Torbjorn SVENSSON <torbjorn.svensson@st.com> writes:
> >
> >> Hello,
> >>
> >> It would have been easier to review the patch if it was inline, but
> >> this will have to do anyway.
> >
> > Hi Torbjorn,
> >
> > sorry most mail readers easily show inline attacchaments of type
> > "text/plain" allowing for inline reply, at the same time this way they
> > can still retain the notion of attached file.  This is how I rutinary
> > sent my patches to other GNU projects (including GCC) so far.  Has
> > newlib some specific rule around this?
> >
> >> I think there is a typo in math.h. Aren't you supposed to do "#ifndef"
> and not "#ifdef"?
> >
> > I guess we are talking about this hunk?
> >
> >   #ifdef __epiphany__
> > diff --git a/newlib/libc/include/math.h b/newlib/libc/include/math.h
> > index ba1a8a17e..da056b5b6 100644
> > --- a/newlib/libc/include/math.h
> > +++ b/newlib/libc/include/math.h
> > @@ -158,6 +158,15 @@ extern int isnan (double);
> >      #else
> >       /* Implementation-defined.  Assume float_t and double_t have been
> >        * defined previously for this configuration (e.g. config.h). */
> > +
> > +   /* If __DOUBLE_TYPE is defined (__FLOAT_TYPE is then supposed to be
> > +      defined as well) float_t and double_t definition is suggested by
> > +      an arch specific header.  */
> > +   #ifdef __DOUBLE_TYPE
> > +    typedef __DOUBLE_TYPE double_t;
> > +    typedef __FLOAT_TYPE float_t;
> > +   #endif
> > +   /* Assume config.h has provided these types.  */
> >     #endif
> >   #else
> >       /* Assume basic definitions.  */
> >
> > I believe the #ifdef is correct.  As the comment suggests if
> > __DOUBLE_TYPE is defined we'll use it to define double_t otherwise we
> > assume is config.h has provided the type definition.
> >
> > I'm reattaching the latest version of this patch with a typo fixed.
> >
> > Thanks!
> >
> >    Andrea
> >
>
> I think the hunks in machine/ieeefp.h warrant a comment as to why we
> can't rely on __FLT_EVAL_METHOD__.  Other than that it LGTM, but you'll
> need Corinna or Jeff to approve.
>
> R.
>
>
Please add the comment as suggested by Richard and it will be pushed.

-- Jeff J.