[libc-commits] [PATCH] D106177: [libc] Add math functions to Windows's entrypoints

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/config/windows/entrypoints.txt:69
+    libc.src.math.modfl
+    libc.src.math.nextafter
+    libc.src.math.nextafterf
----------------
Can you add `nextafter` functions in a different patch along with the fix in `NextAfterTest.h`.


================
Comment at: libc/test/src/math/NextAfterTest.h:26
   static constexpr int bitWidthOfType =
-      __llvm_libc::cpp::IsSame<T, long double>::Value ? 80 : (sizeof(T) * 8);
-#else
-  static constexpr int bitWidthOfType = sizeof(T) * 8;
-#endif
+      __llvm_libc::fputil::FloatProperties<T>::bitWidth;
 
----------------
As pointed out above, can you move this to a separate patch?


================
Comment at: libc/utils/FPUtil/x86_64/FEnv.h:356
   internal::getX87StateDescriptor(state->X87Status);
+#if !(defined(_WIN32) || defined(_WIN64))
   state->MXCSR = internal::getMXCSR();
----------------
Leave this out as this is not the correct fix. We can make another pass, when we enable the fenv functions, to fix these up correctly.


================
Comment at: libc/utils/FPUtil/x86_64/FEnv.h:366
   internal::writeX87StateDescriptor(state->X87Status);
+#if !(defined(_WIN32) || defined(_WIN64))
   internal::writeMXCSR(state->MXCSR);
----------------
Same here.


================
Comment at: libc/utils/FPUtil/x86_64/FEnv.h:94
   // TODO: Elaborate the remaining 20 bytes as required.
+  #if !(defined(_WIN32) || defined(_WIN64))
   uint32_t _[5];
----------------
aeubanks wrote:
> this change isn't obvious and isn't described in the commit description, it'd be nice to add why this change was made
Can you split this into a different patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106177