This is an archive of the discontinued LLVM Phabricator instance.

[libc] Fix 64-bit ARM support and header includes
ClosedPublic

Authored by ddcc on Nov 19 2021, 5:19 AM.

Diff Detail

Event Timeline

ldionne created this revision.Nov 19 2021, 5:19 AM
ldionne requested review of this revision.Nov 19 2021, 5:19 AM
kristof.beyls added inline comments.Nov 19 2021, 5:33 AM
libc/src/__support/FPUtil/PlatformDefs.h
18–19

Apologies for the drive-by comment.
I just noticed the spelling arch64. It looks like maybe this should be aarch64?

Thanks for catching the build problems with exhaustive tests. They have bit rotted a bit because we don't run them on any bot.

libc/src/__support/FPUtil/PlatformDefs.h
18–19

On aarch64, long doubles are not the 64-bit IEEE double precision numbers. So, this would be incorrect after the typo is fixed.

libc/test/src/math/exhaustive/cosf_test.cpp
12

You should not need these headers to be included at all.

ddcc commandeered this revision.Jan 27 2022, 5:04 PM
ddcc added a reviewer: ldionne.
ddcc added a subscriber: ddcc.

Sorry for the delay, Louis was helping me upstream this but now I can update it directly.

ddcc updated this revision to Diff 403840.Jan 27 2022, 5:05 PM

Clarify long double is for apple arm 64-bit

ddcc edited reviewers, added: sivachandra; removed: ldionne.Jan 27 2022, 5:06 PM

Clarify long double is for apple arm 64-bit

LGTM, but is there any document that can be added as a reference in a comment in PlatformDefs.h?

ddcc updated this revision to Diff 403909.Jan 28 2022, 12:18 AM

Add apple reference

sivachandra accepted this revision.Jan 28 2022, 12:21 AM
This revision is now accepted and ready to land.Jan 28 2022, 12:21 AM
This revision was landed with ongoing or failed builds.Jan 28 2022, 12:22 AM
This revision was automatically updated to reflect the committed changes.