Re: [PATCH v6] Add i386 and x86_64 fenv support from Cygwin.

On Mon, Sep 30, 2019 at 10:30 AM Howland, Craig D. - US via newlib
<[email protected]> wrote:
>
> > From: [email protected] <[email protected]> on behalf of Sebastian Huber <[email protected]>
> > Sent: Monday, September 30, 2019 2:03 AM
> > To: [email protected]
> > Subject: [PATCH v6] Add i386 and x86_64 fenv support from Cygwin.
> >
> > From: Joel Sherrill <[email protected]>
> >
> > ---
> > v6:
> >
> > Added files generated by Automake 1.11.6 and Autoconf 2.68.  The i386-rtems
> > and x86_64-rtems targets build fine now.
> >
> >  ...
> >  newlib/libm/machine/i386/feclearexcept.c     |     1 +
> >  newlib/libm/machine/i386/fegetenv.c          |     1 +
> >  newlib/libm/machine/i386/fegetexceptflag.c   |     1 +
> >  newlib/libm/machine/i386/fegetround.c        |     1 +
> >  newlib/libm/machine/i386/feholdexcept.c      |     1 +
> >  newlib/libm/machine/i386/fenv.c              |     1 +
> >  newlib/libm/machine/i386/feraiseexcept.c     |     1 +
> >  newlib/libm/machine/i386/fesetenv.c          |     1 +
> >  newlib/libm/machine/i386/fesetexceptflag.c   |     1 +
> >  newlib/libm/machine/i386/fesetround.c        |     1 +
> >  newlib/libm/machine/i386/fetestexcept.c      |     1 +
> >  newlib/libm/machine/i386/feupdateenv.c       |     1 +
> >  ...
> >  newlib/libm/machine/x86_64/feclearexcept.c   |     1 +
> >  newlib/libm/machine/x86_64/fegetenv.c        |     1 +
> >  newlib/libm/machine/x86_64/fegetexceptflag.c |     1 +
> >  newlib/libm/machine/x86_64/fegetround.c      |     1 +
> >  newlib/libm/machine/x86_64/feholdexcept.c    |     1 +
> >  newlib/libm/machine/x86_64/fenv.c            |   477 +
> >  newlib/libm/machine/x86_64/feraiseexcept.c   |     1 +
> >  newlib/libm/machine/x86_64/fesetenv.c        |     1 +
> >  newlib/libm/machine/x86_64/fesetexceptflag.c |     1 +
> >  newlib/libm/machine/x86_64/fesetround.c      |     1 +
> >  newlib/libm/machine/x86_64/fetestexcept.c    |     1 +
> >  newlib/libm/machine/x86_64/feupdateenv.c     |     1 +
> >  39 files changed, 16486 insertions(+), 13 deletions(-)
>      It seems rather odd to be adding a new fenv.c file which is not in the
> main/template (libm/fenv) directory which has all the functions in it and then
> providing empty versions of the individually-named function files.  (I see
> that is was this way in v5 of the patch, too, but I had not gotten a chance
> to look at that one.)  They had purposely been split into separate files when
> the template was made, with one of the reasons being link size.  This defeats
> that one purpose, although that purpose is perhaps of limited value for i386
> and x84_64 targets.  In a quick glance in fenv.c, there does not appear to be
> a reason (e.g.  shared statics) that would "require" them to be combined.

I don't think we want to modify any source code that comes from a third
party. This is a very lightly modified gwcopy of the Cygwin implementation.
I expect those folks will switch to this when Corinna gets back from holiday.
If she wants to split it, I'm OK with it.

But I plan to port implementations from various places and merge with minimal
modifications. Even if this target gets split, I don't think we will
end up splitting
the others.

Over at RTEMS, we call the attribute describing the ability to track source code
back to its original source "source transparency". You don't want to hurt source
transparency for fairly obvious maintenance reasons.

For size, it likely doesn't matter on this target (or any application
using fenv) But
you can use per function section linking and not worry about one
method/symbol per file.

I think we are agreeing to not worry about this one but it foreshadows
this happening
again on other architectures.

--joel

>                                 Craig
>
>