Skip to content

Make openlibm less dependent on the host, but still add knobs to build against it#77

Merged
ViralBShah merged 9 commits intoJuliaMath:masterfrom
NuxiNL:system-fenv
Jan 10, 2015
Merged

Make openlibm less dependent on the host, but still add knobs to build against it#77
ViralBShah merged 9 commits intoJuliaMath:masterfrom
NuxiNL:system-fenv

Conversation

@EdSchouten
Copy link
Copy Markdown
Contributor

Hi there,

First of all, thank you guys for being so responsive to all of my previous pull requests. Well appreciated!

Attached is a somewhat largish patchset that essentially decouples OpenLibm from the host system. It patches the code up in such a way that it no longer includes <fenv.h>, <complex.h> and <math.h>. Instead, it will use <openlibm_fenv.h>, <openlibm_complex.h> and <openlibm.h>. <openlibm_complex.h> has been patched up to become feature-complete.

I've added some bits to the <openlibm*.h> to switch building against the host headers. I can use this switch to build the math library for the POSIX-like environment I'm working on.

Thoughts?

Ed

[Edit: Fix formatting for the headers, they didn't show up for me otherwise - @Keno]

OpenLibm has an implementation of fenv.h internally. This may be
problematic in case you want it to build against the host system's
implementation, as it would require you to somehow take the fenv.h file
out of the compiler search path.

Simply use a different naming scheme, similar to openlibm.h and
openlibm_complex.h. If we want to build against the host's fenv.h, we
can simply add an '#include <fenv.h>' from within this header.
If we ship with a separate copy of <complex.h>, we should also use this
one instead of the one provided by the system.
We already use this construct in cabs() to call hypot(), so I guess we
can assume our targeted compilers support this construct.
…mplex.h>.

While there, make CMPLX() work with Clang by using compound literals.
Now that cimag*() uses __imag__, we can also just inline the unions.
There is no need for the separate types anymore.

Also just define CMPLX() unconditionally now, as we no longer pull in
the host's <complex.h>.
signbit() and friends should always take floating point arguments. This
fixes a compiler error when using FreeBSD's own <math.h>.
Put external headers before internal ones. While there, replace a lot of
occurences of "openlibm.h" with <openlibm.h>. It should be thought of as
an external header, as it is installed along with the library.
We can now simply use -DOPENLIBM_USE_HOST_*_H to do this.
@tkelman
Copy link
Copy Markdown
Contributor

tkelman commented Jan 9, 2015

We'll need to test this along with Julia's build system. Why did you choose to use the changed behavior by default? If for some reason this new changed self-contained mode doesn't work for Julia it would be less invasive for us to have the self-contained mode be the exceptional non-default case.

@EdSchouten
Copy link
Copy Markdown
Contributor Author

Hello Tony,

That's a very good question. I think it really depends on how we see OpenLibm as an individual project (not specifically focussing on Julia's use-case). As an outsider, my guess would be that there are two use-cases for OpenLibm:

  1. As a separate library that can be used next to the system's math library.
  2. As the actual math library of the system.

The question is which use-case we consider to be the default. If I check out the OpenLibm code and build it with the default options, I would personally expect it would be targeted at the first use-case. This is also how related libraries such as jemalloc (http://www.canonware.com/jemalloc/) work. It will simply install a jemalloc.h, containing functions like je_malloc(), je_free(), etc. People who want to use jemalloc or a math library as the core implementation of their operating system are likely the ones who mind don't getting their hands dirty.

That said, I personally don't care about the default behaviour. If you want, I can add the -DOPENLIBM_USE_HOST_*_H flags to the default CFLAGS.

Out of curiosity, how does the Julia language exactly use OpenLibm? Does the Julia runtime explicitly include <openlibm.h>, or is built against the system's <math.h>, hoping that OpenLibm provides the functions using the same ABI?

Ed

@tkelman
Copy link
Copy Markdown
Contributor

tkelman commented Jan 9, 2015

That's a very good point, mostly curious on your reasoning more than disagreeing with it. Also curious to find out more about the project you're working on.

By default Julia builds and calls into openlibm, but there's an option in our build system to link against system libm if you so choose. We've found a lot of variation in performance, accuracy, and correctness across different platforms' respective libm implementations, so nicer to have something uniform and predictable.

@ViralBShah
Copy link
Copy Markdown
Member

@EdSchouten Julia dlopen's and calls the functions in openlibm - so it does not care about openlibm.h.

This is the right set of patches - we really do not wish to depend on math.h. I do think that using the system provided headers is a good default - but I'd like to try that out on different systems for a while, before we make it default. Could you revert just that bit for now?

I'd like a separate PR for using system headers the default, where we can discuss the outcomes on various OSes, systems, and such.

@tkelman
Copy link
Copy Markdown
Contributor

tkelman commented Jan 9, 2015

FYI we also statically link libopenlibm.a into julia.exe on Windows.

@EdSchouten
Copy link
Copy Markdown
Contributor Author

Wow. I just discovered something interesting: it seems that OpenLibm already never depended on the <fenv.h> and <math.h> headers of the host system. It only included the <complex.h> header from the host system. Very confusing.

The only places where we included the host system's <math.h> is in functions in ld128/ that are not yet hooked up to the build yet. That said, including <complex.h> of the host system was quite harmless. This header essentially only defines _Complex_I, CMPLX*() and function prototypes. These have now been moved into <openlibm_complex.h>.

So to summarize: no behaviour has been changed substantially. The only thing I've done is made it more consistent (we no longer use the host system's <complex.h> ) and extended it to at least give the option to build against the host system's headers, which is needed for my specific use-case. The changes in this pull request could be merged as is.

I hope this explanation is clear enough. If not, be sure to get in touch.

Thanks,
Ed

ViralBShah added a commit that referenced this pull request Jan 10, 2015
Make openlibm less dependent on the host, but still add knobs to build against it
@ViralBShah ViralBShah merged commit 89ac4d4 into JuliaMath:master Jan 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants