[libc-commits] [PATCH] D145099: [libc] add basic Intel MacOS configuration

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/FPUtil/FEnvImpl.h:26
+#elif defined(LIBC_TARGET_ARCH_IS_X86) && !defined(__APPLE__)
+#if defined(__APPLE__)
+// TODO: add darwin x86_64 fenv
----------------
sivachandra wrote:
> The `!defined(__APPLE__)` above and this condition here are opposites? Irrespective, can we use a model like the `AARCH64` one?
> 
> ```
> #elif defined(LIBC_TARGET_ARCH_IS_X86)
> #if defined(__APPLE__)
> #error ...
> #else
> #include <x86_64/FEnvImpl.h>
> #endif
> #endif
> ```
The reason I have it like this for the moment is because I want it to use the dummy functions, and for that to work it needs to pass over both the aarch64 and x86_64 conditions. In future I'd like to have a specific fenv implementation for darwin x86_64, but that's beyond the scope of this patch. I've added a comment to explain.

I can't tell it to error if it detects x86_64 MacOS, since there are some tests that aren't part of any entrypoint but include fenv, specifically the printf_core converter tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145099/new/

https://reviews.llvm.org/D145099