Re: [PATCH v5] rtl: builtins: (not just) rs6000: Add builtins for fegetround, feclearexcept and feraiseexcept [PR94193]

[Raoni Fassina Firmino <raoni@linux.ibm.com>]

It seems to me we have two unrelated concerns mixed in the threads, I
will reply in two different sub-threads to make this easier.

This one to discuss the values of FE_* macros.


On Tue, Nov 17, 2020 at 03:23:02PM -0700, AL gcc-patches wrote:
> >>> +@cindex @code{fegetround@var{m}} instruction pattern
> >>> +@item @samp{fegetround@var{m}}
> >>> +Store the current machine floating-point rounding mode into operand 0.
> >>> +Operand 0 has mode @var{m}, which is scalar.  This pattern is used to
> >>> +implement the @code{fegetround} function from the ISO C99 standard.
> >> I think this needs to elaborate on the format of the "rounding mode".
> >>
> >> AFAICS you do nothing to marshall with the actually used libc
> >> implementation which AFAIU can choose arbitrary values for
> >> the FE_* macros.  I'm not sure we require the compiler to be
> >> configured for one specific C library and for example require
> >> matching FE_* macro definitions for all uses of the built
> >> compiler.
> >>
> >> For the patch at hand you seem to assume the libc "format"
> >> matches the hardware one (which would of course be reasonable).
> >>
> >> Does that actually hold up when looking at libcs other than 
> >> glibc supporting powerpc?
> > I checked in some other libc implementations that have POWER support and
> > all have the same value as glic for the four rounding modes and the five
> > exception flags from libc. The libcs implementations I checked are:
> >
> >  - musl
> >  - uclibc & uclibc-ng
> >  - freebsd
> >
> > Is There any other I am missing?
> I think the concern here is that while the libcs we have visibility into
> have consistent values, I don't think that's necessarily guaranteed.  
> I'm not sure how to DTRT here.  We may not have the target headers if
> we're doing a cross compile, so a configure test may not do what we 
> need.  In fact, ISTM that there is no reliable configure or compile time
> check we can do since the constants are part of the runtime and can
> change independently of the compiler.


From other subthreads Joseph and segher mentioned this:

On Wed, Nov 04, 2020 at 09:06:02PM +0000, Joseph Myers wrote:
> On Wed, 4 Nov 2020, Raoni Fassina Firmino via Gcc-patches wrote:
> 
> > IMHO, It seems like it is not necessary if there not a libc that have
> > different values for the FE_* macros. I didn't check other archs, but if
> > is the case for some other arch I think it could be changed if and when
> > some other arch implements expands for these builtins.
> 
> SPARC is the case I know of where the FE_* values vary depending on target 
> libc (see the SPARC_LOW_FE_EXCEPT_VALUES target macro).

On Wed, Nov 18, 2020 at 06:38:22AM -0600, Segher Boessenkool wrote:
> We can handle the constants issue similarly to what we do for
> __builtin_fpclassify, too.


I think that if we must safe-guard for future or unforeseen libc
implementations doing what __builtin_fpclassify does is the way to go.
I don't know what is the GCC police here, but IMHO I don't think we
should add complexity before it is needed in this case.  And looking at
__builtin_fpclassify, it seems a lot, IIUC this solution needs
fixinclude to work, seems to me too much add maintenance for something
that is not needed yet, because SPARC don't have this expands, none has
for now.

I don't know if it helps, but the included tests also check the values
changes against the libc implementations, so may catch discrepancies if
building gcc with other libcs.  It, of course, doesn't help if using for
example a gcc built with glibc and compiling a program with it with some
unknown libc.  I wonder if some safe check that can be done at runtime,
whilst building a program with gcc and some unknown libc.


o/
Raoni Fassina