Re: [PATCH V2] Aarch32/64: Support __FLT_EVAL_METHOD__ values other than 0, 1, 2
Richard Earnshaw <[email protected]>
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <[email protected]> |
On 04/04/2022 15:53, Andrea Corallo wrote: > Torbjorn SVENSSON <[email protected]> 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.