This is an archive of the discontinued LLVM Phabricator instance.

[libc] Initial support for darwin-aarch64.
ClosedPublic

Authored by lntue on Mar 3 2022, 9:10 AM.

Details

Summary

Add initial support for darwin-aarch64 (macOS M1).

Some differences compared to linux-aarch64:

  • math.h defined math_errhandling by the compiler builtin __math_errhandling() but Apple Clang 13.0.0 on M1 does not support __math_errhandling() builtin as a macro function or a constexpr function.
  • math.h defines UNDERFLOW and OVERFLOW macros.
  • Besides 5 usual floating point exceptions: FE_INEXACT, FE_UNDERFLOW, FE_OVERFLOW, FE_DIVBYZERO, and FE_INVALID, fenv.h also has another floating point exception: FE_FLUSHTOZERO. The corresponding trap for FE_FLUSHTOZERO in the control register is at the different location compared to the status register.
  • FE_FLUSHTOZERO exception flag cannot be raised with the default CPU floating point operation mode.

Diff Detail

Event Timeline

lntue created this revision.Mar 3 2022, 9:10 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 3 2022, 9:10 AM
lntue requested review of this revision.Mar 3 2022, 9:10 AM
lntue edited the summary of this revision. (Show Details)Mar 3 2022, 9:13 AM

LGTM but Siva should probably approve.

libc/test/src/fenv/feclearexcept_test.cpp
27 ↗(On Diff #412742)

I think removing the other tests is okay, but probably copy the comments that used to be on the first test into the new only test.

This is a large change mixing up independent cleanups with directed changes. Can you please split this up into separate patches so that it is easy to see what and why? Considering that we do not break anyone, you can do it in many small changes to keep the discussion/questions clearly demarcated.

lntue added a comment.Mar 4 2022, 2:37 PM

This is a large change mixing up independent cleanups with directed changes. Can you please split this up into separate patches so that it is easy to see what and why? Considering that we do not break anyone, you can do it in many small changes to keep the discussion/questions clearly demarcated.

The changes on FEnvUtils. and aarch64/FEnvImpl.h were splitted to https://reviews.llvm.org/D120965 and https://reviews.llvm.org/D120997

lntue marked an inline comment as done.Mar 4 2022, 2:38 PM

Can I trouble with another request? Adding conditionals for errno and fp exception testing is setting a bad precedence. Can we instead add convenience macros EXPECT_MATH_ERRNO and EXPECT_MATH_FPEXCEPT and separate out those changes from this patch?

libc/src/__support/common.h
22–23

Can you add a detailed comment here explaining why __APPLE__ be excluded? Also talk about its implications.

libc/test/src/stdlib/strtold_test.cpp
178

Why are the changes in this file required?

lntue added a comment.Mar 8 2022, 10:39 AM

Can I trouble with another request? Adding conditionals for errno and fp exception testing is setting a bad precedence. Can we instead add convenience macros EXPECT_MATH_ERRNO and EXPECT_MATH_FPEXCEPT and separate out those changes from this patch?

I added testing macros for ERRNO and floating point exceptions in https://reviews.llvm.org/D121235

lntue updated this revision to Diff 413950.Mar 8 2022, 3:10 PM

Sync and address comments.

lntue marked an inline comment as done.Mar 8 2022, 3:15 PM
lntue added inline comments.
libc/test/src/stdlib/strtold_test.cpp
178

These are corresponding outputs and errno's if the output type is double precision. I guess we just haven't tested these when long double is double yet.

michaelrj added inline comments.Mar 8 2022, 3:50 PM
libc/test/src/stdlib/strtold_test.cpp
178

Tue is correct, I didn't have a long double is double system to test with when I was originally writing these tests. His changes are correct, as checked by calling strtod on those inputs.

sivachandra accepted this revision.Mar 10 2022, 1:30 AM
sivachandra added inline comments.
libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
2

A large part of this seems to be the same as the default aarch64/FEnvImpl.h. In a later pass, we should unify as much as possible so that the differences get clearly documented.

This revision is now accepted and ready to land.Mar 10 2022, 1:30 AM

Also, can you edit the commit message so that it sticks to the 80-character limit?

This revision was automatically updated to reflect the committed changes.