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

[Corinna Vinschen <vinschen at redhat dot com>]

On Sep  4 15:10, Joel Sherrill wrote:
> Hi
> 
> I thought I was close to wrapping this up but I have run into a couple of issues
> with this implementation being a single file and the stub being a collection
> of files. After my most recent rebuild, I am getting linking conflicts because
> the implementation doesn't override the stubs.
> 
> This means that each implementation needs to be multiple files with the
> same names as the stub implementation. The riscv does this.but the x86
> does not.
> 
> (1) Do you all agree that I need split the single fenv.c file into
> multiple files?
> Or should I provide empty versions of each file in libm/fenv so an
> implementation
> can be one file but just override the other files with common files. I
> ask because
> I suspect other architecture implementations will also be one file or inline the
> implementation in sys/fenv.h. This would make it easy to merge those.
> 
> Suggestions?

Ideally all targets define one function per file.  Cygwin is probably
the only target here which does not care for space, but it doesn't hurt
if the functions are split for Cygwin either.  So if only the actually
required functions are linked into an executable, you won't find anybody
complaining.

> (1a) If I do split, do I provide a file like fenvimpl.h for the constants at the
> top of the file? And place it as a local file in libm/machine/i386 and x86_64?

In theory this should not be necessary.  sys/fenv.h is getting overridden
by the target-specific implementation, that should be sufficient.

> (2) Independent of splitting the file, I noticed use_sse is used in multiple
> methods but only set by _feinitialize(). I THINK the solution is to turn
> use_sse into a method which dynamically checks the cpuid feature flag.
> We can't rely on that always being called.

Makes sense.  Please note that Jeff will take over from here.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: signature.asc
Description: PGP signature