This is an archive of the discontinued LLVM Phabricator instance.

libc++: add _LIBCPP_HAS_NO_LONG_DOUBLE
AbandonedPublic

Authored by deeglaze on Jul 2 2019, 2:32 PM.

Details

Reviewers
jroelofs
EricWF
Summary

Add a symbol similar to _LIBCPP_HAS_NO_LONG_LONG to allow compatibility with newlib when _LDBL_EQ_DBL is not defined. One such platform where this is the case is the POSIX-compatible enclave development platform, Asylo.

Diff Detail

Event Timeline

deeglaze created this revision.Jul 2 2019, 2:32 PM
deeglaze created this object with edit policy "Subscribers".
jfb added a subscriber: mclow.lists.Jul 2 2019, 2:38 PM
jfb added a subscriber: jfb.
deeglaze updated this revision to Diff 207636.Jul 2 2019, 2:39 PM

Changed an instance of defined(ASYLO) to defined(_LIBCXX_HAS_NO_LONG_DOUBLE) in math.h

deeglaze retitled this revision from libc++: add _LIBCXX_HAS_NO_LONG_DOUBLE to libc++: add _LIBCPP_HAS_NO_LONG_DOUBLE.Jul 2 2019, 2:40 PM
deeglaze edited the summary of this revision. (Show Details)
jfb added inline comments.Jul 2 2019, 2:41 PM
libcxx/include/math.h
789

These seem unfortunate. Can you look at the history to see why there's no feature macro that explains with?

deeglaze marked an inline comment as done.Jul 2 2019, 2:51 PM
deeglaze added inline comments.
libcxx/include/math.h
789

It's not a uniform feature for Sun and AIX, as you can see further down for declarations like asinh. The sun support came 4 years ago in fbbfd092 for cmath, since math.h split from cmath after that. The AIX support was 6 years ago in 5d1a701, which seems slightly entangled with Microsoft's C runtime (_LIBCPP_MSVCRT). I'm not familiar enough with this codebase to suggest a feature strategy to overcome these unexpectedly non-uniform distinctions.

jfb added inline comments.Jul 2 2019, 2:55 PM
libcxx/include/math.h
789

Ugh yeah that sounds like a mess not worth touching in this patch. Thanks for looking into it.

This patch seems incomplete to me. There's stuff in <type_traits> like is_floating_point, etc.

libcxx/include/cmath
445

These seem wrong to me. nexttoward takes a double, and nexttowardf takes a float.

libcxx/include/cstdlib
114

There's a lot more work to be done if you want to remove strtold.
Check out:

  • src/string.cpp
  • include/stdlib.h
  • several other places - do a search
libcxx/include/locale
180

Where did this come from?

miyuki added a subscriber: miyuki.Jul 3 2019, 2:35 AM
miyuki added a comment.Jul 4 2019, 3:38 AM

We have a similar problem with our port of libc++ and we also had to conditionalize all uses of long double downstream. Here is a list of files that we had to change: https://pastebin.com/5tTcSYxW
As you see, most of them are tests.
Another problem with long double is that it is used in a couple of places not strictly requiring it (include/locale, include/__mutex_base and include/thread), in those cases we introduced a _LIBCPP_LONGEST_DOUBLE macro which is defined to either double or long double depending on the availability of the latter.

deeglaze abandoned this revision.Jul 17 2019, 12:13 PM
deeglaze marked 4 inline comments as done.

After further experimentation in our toolchain, it is infeasible for us to avoid the long double maths functions. We will just have to supplement them.

libcxx/include/cmath
445

They both take 2 arguments, the second of which is a long double. Neither are defined in newlib if _LDBL_EQ_DBL is not defined.

libcxx/include/locale
180

Removed.