[libc-commits] [libc] 9474ddc - [libc] Fix feclearexcept for x86_64.

[Siva Chandra Reddy via libc-commits]

Author: Siva Chandra Reddy
Date: 2021-06-30T17:28:06Z
New Revision: 9474ddc3ac8637596f87dd796864353317622672

URL: https://github.com/llvm/llvm-project/commit/9474ddc3ac8637596f87dd796864353317622672
DIFF: https://github.com/llvm/llvm-project/commit/9474ddc3ac8637596f87dd796864353317622672.diff

LOG: [libc] Fix feclearexcept for x86_64.

Previously, feclearexcept cleared all exceptions irrespective of the
argument. This change brings it in line with the aarch64 flavors wherein
only those exceptions listed in the argument will be cleared.

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D105081

Added: 
    libc/test/src/fenv/feclearexcept_test.cpp

Modified: 
    libc/test/src/fenv/CMakeLists.txt
    libc/utils/FPUtil/x86_64/FEnv.h

Removed: 
    


################################################################################
diff  --git a/libc/test/src/fenv/CMakeLists.txt b/libc/test/src/fenv/CMakeLists.txt
index 851db03985698..1fa3e687e5cbb 100644
--- a/libc/test/src/fenv/CMakeLists.txt
+++ b/libc/test/src/fenv/CMakeLists.txt
@@ -60,6 +60,17 @@ add_libc_unittest(
     libc.utils.FPUtil.fputil
 )
 
+add_libc_unittest(
+  feclearexcept_test
+  SUITE
+    libc_fenv_unittests
+  SRCS
+    feclearexcept_test.cpp
+  DEPENDS
+    libc.src.fenv.feclearexcept
+    libc.utils.FPUtil.fputil
+)
+
 if (NOT LLVM_USE_SANITIZER)
   # Sanitizers don't like SIGFPE. So, we will run the 
   # tests which raise SIGFPE only in non-sanitizer builds.

diff  --git a/libc/test/src/fenv/feclearexcept_test.cpp b/libc/test/src/fenv/feclearexcept_test.cpp
new file mode 100644
index 0000000000000..dfdc29dc85fba
--- /dev/null
+++ b/libc/test/src/fenv/feclearexcept_test.cpp
@@ -0,0 +1,83 @@
+//===-- Unittests for feclearexcept with exceptions enabled ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/fenv/feclearexcept.h"
+
+#include "utils/FPUtil/FEnv.h"
+#include "utils/UnitTest/Test.h"
+
+#include <fenv.h>
+#include <stdint.h>
+
+TEST(LlvmLibcFEnvTest, ClearTest) {
+  uint16_t excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
+                        FE_UNDERFLOW};
+  __llvm_libc::fputil::disableExcept(FE_ALL_EXCEPT);
+  __llvm_libc::fputil::clearExcept(FE_ALL_EXCEPT);
+
+  for (uint16_t e : excepts)
+    ASSERT_EQ(__llvm_libc::fputil::testExcept(e), 0);
+
+  __llvm_libc::fputil::raiseExcept(FE_ALL_EXCEPT);
+  for (uint16_t e : excepts) {
+    // We clear one exception and test to verify that it was cleared.
+    __llvm_libc::feclearexcept(e);
+    ASSERT_EQ(uint16_t(__llvm_libc::fputil::testExcept(FE_ALL_EXCEPT)),
+              uint16_t(FE_ALL_EXCEPT & ~e));
+    // After clearing, we raise the exception again.
+    __llvm_libc::fputil::raiseExcept(e);
+  }
+
+  for (uint16_t e1 : excepts) {
+    for (uint16_t e2 : excepts) {
+      __llvm_libc::feclearexcept(e1 | e2);
+      ASSERT_EQ(uint16_t(__llvm_libc::fputil::testExcept(FE_ALL_EXCEPT)),
+                uint16_t(FE_ALL_EXCEPT & ~(e1 | e2)));
+      __llvm_libc::fputil::raiseExcept(e1 | e2);
+    }
+  }
+
+  for (uint16_t e1 : excepts) {
+    for (uint16_t e2 : excepts) {
+      for (uint16_t e3 : excepts) {
+        __llvm_libc::feclearexcept(e1 | e2 | e3);
+        ASSERT_EQ(uint16_t(__llvm_libc::fputil::testExcept(FE_ALL_EXCEPT)),
+                  uint16_t(FE_ALL_EXCEPT & ~(e1 | e2 | e3)));
+        __llvm_libc::fputil::raiseExcept(e1 | e2 | e3);
+      }
+    }
+  }
+
+  for (uint16_t e1 : excepts) {
+    for (uint16_t e2 : excepts) {
+      for (uint16_t e3 : excepts) {
+        for (uint16_t e4 : excepts) {
+          __llvm_libc::feclearexcept(e1 | e2 | e3 | e4);
+          ASSERT_EQ(uint16_t(__llvm_libc::fputil::testExcept(FE_ALL_EXCEPT)),
+                    uint16_t(FE_ALL_EXCEPT & ~(e1 | e2 | e3 | e4)));
+          __llvm_libc::fputil::raiseExcept(e1 | e2 | e3 | e4);
+        }
+      }
+    }
+  }
+
+  for (uint16_t e1 : excepts) {
+    for (uint16_t e2 : excepts) {
+      for (uint16_t e3 : excepts) {
+        for (uint16_t e4 : excepts) {
+          for (uint16_t e5 : excepts) {
+            __llvm_libc::feclearexcept(e1 | e2 | e3 | e4 | e5);
+            ASSERT_EQ(uint16_t(__llvm_libc::fputil::testExcept(FE_ALL_EXCEPT)),
+                      uint16_t(FE_ALL_EXCEPT & ~(e1 | e2 | e3 | e4 | e5)));
+            __llvm_libc::fputil::raiseExcept(e1 | e2 | e3 | e4 | e5);
+          }
+        }
+      }
+    }
+  }
+}

diff  --git a/libc/utils/FPUtil/x86_64/FEnv.h b/libc/utils/FPUtil/x86_64/FEnv.h
index f654f0bab2d51..cd11c388f41f4 100644
--- a/libc/utils/FPUtil/x86_64/FEnv.h
+++ b/libc/utils/FPUtil/x86_64/FEnv.h
@@ -188,12 +188,10 @@ static inline int disableExcept(int excepts) {
 }
 
 static inline int clearExcept(int excepts) {
-  // An instruction to write to x87 status word ins't available. So, we
-  // just clear all of the x87 exceptions.
-  // TODO: One can potentially use fegetenv/fesetenv to clear only the
-  // listed exceptions in the x87 status word. We can do this if it is
-  // really required.
-  internal::clearX87Exceptions();
+  internal::X87StateDescriptor state;
+  internal::getX87StateDescriptor(state);
+  state.StatusWord &= ~internal::getStatusValueForExcept(excepts);
+  internal::writeX87StateDescriptor(state);
 
   uint32_t mxcsr = internal::getMXCSR();
   mxcsr &= ~internal::getStatusValueForExcept(excepts);