[flang] Fix setting mxcsr on MSVC

[ro]

I wonder if something like this patch should be applied more broadly or even in general. I'm seeing the same issue (lack of __mxcsr in fenv_t)
on Solaris/x86. When looking around, I found that that non-standard member does exist in glibc, macOS, and FreeBSD, so they could continue
to use the existing code.

However, AFAICS <xmmintrin.h> is available with all compilers that can be used to build llvm: clang, gcc, and msvc, so one could just remove
the references to __mxcsr and use the _mm_getcsr/_mm_setcsr code instead.

Thoughts?

[isuruf]

I agree that we can use it for all platforms. @sscalpone, what do you think?

[sscalpone]

Yes, _mm_setcsr seems portable across x86_64 systems. It's fine. Is there an already-existing llvm API that is similar? (I don't know.)

[isuruf]

Use _mm_getcsr/_mm_setcsr in all x86_64

[isuruf]

Remove MSVC mentions

[sscalpone]

This change doesn't preserve the meaning of currentFenv_. I don't know if currentFenv_ (or originalFenv_) are still useful.

[isuruf]

Thanks @sscalpone for the review. Looks like originalFenv_ is still used for restoring the original state. Shall I change the type of originalFenv_ to be unsigned int on x86_64? (Or maybe a union?)
From what I understand originalFenv_ is used for changing and restoring the __mxcsr value (or its equivalents on aarch64).

[jeanPerier]

Looks like originalFenv_ is still used for restoring the original state. Shall I change the type of originalFenv_ to be unsigned int on x86_64? (Or maybe a union?)
From what I understand originalFenv_ is used for changing and restoring the __mxcsr value (or its equivalents on aarch64).

In host.cpp, originalFenv_ is more than just the __mxcsr, it also backs-up the whole fp environment (original rounding mode, exceptions that were raised,...) before calling the run-time to fold. Interrupts are also disabled between the related feholdexcept / fesetenv (we do not want the compiler to crash if the Fortran code being folded is wrong and triggers a division by zero or such), so it should be kept.
currentFenv_ however was only about manipulating the __mxcsr (or __fpcr /...), so you can remove it where you do not need it anymore.

[ro]

I just tried a 2-stage build on amd64-pc-solaris2.11, which failed:

FAILED: tools/flang/unittests/Evaluate/CMakeFiles/FortranEvaluateTesting.dir/fp-testing.cpp.o
[...]
In file included from /vol/llvm/src/llvm-project/local/flang/unittests/Evaluate/fp-testing.cpp:1:
/vol/llvm/src/llvm-project/local/flang/unittests/Evaluate/fp-testing.h:22:10: error: private field 'currentFenv_' is not used [-Werror,-Wunused-private-field]
  fenv_t currentFenv_;
         ^
1 error generated.

[isuruf]

Add a new field originalMxcsr and restore it along with originalFenv

[isuruf]

Refactor

[ro]

Given that this fixes the flang build on at least Windows and Solaris/x86, it would be good if someone could review this.

FWIW, I`ve successfully built and tested with this patch on amd64-pc-solaris2.11

[ro]

It's been two months since the last reminder without any response. Could someone please review this patch? The current code is unportable and just happens to work on a few select x86 targets. It would be a shame to release LLVM 11 with the flang build broken on at least Windows and Solaris/x86.

[jeanPerier]

LGTM, thanks for doing this.

[ro]

@isuruf, do you intend to commit your patch any time soon? It would be good to have it in-tree before LLVM 11 branches.

[isuruf]

@ro, there are some red pre-merge checks. Not sure what those are about. Any ideas?

[ro]

The one from April wasn't run at all, but only reports some obscure internal error. There were generic problems with the pre-merge infrastructure at that time IIRC.

Last night, I tried to restart the check, but it's still stuck in the first step (creating a branch in the repo to apply the patch to) after 10 hours.
In that form, the pre-merge checks are worse than useless IMO.

I'd suggest you go ahead and commit the patch. Should there be any issues despite your and my testing, the buildbots will let us know quickly enough.

[richard.barton.arm]

+1 on @ro 's suggestion.

flang/lib/Evaluate/host.h

public:
   bool hasSubnormalFlushingHardwareControl() const {
     return hasSubnormalFlushingHardwareControl_;
   }
   void SetFlag(RealFlag flag) { flags_.set(flag); }
   bool hardwareFlagsAreReliable() const { return hardwareFlagsAreReliable_; }
 
 private:
   std::fenv_t originalFenv_;
-  std::fenv_t currentFenv_;
+#if __x86_64__
+  unsigned int originalMxcsr;
+#endif
   RealFlags flags_;
   bool hasSubnormalFlushingHardwareControl_{false};
   bool hardwareFlagsAreReliable_{true};
 };
 
 // Type mapping from F18 types to host types
 struct UnsupportedType {}; // There is no host type for the F18 type
 

flang/lib/Evaluate/host.cpp

 //===-- lib/Evaluate/host.cpp ---------------------------------------------===//
 //
 // 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 "host.h"
 
 #include "flang/Common/idioms.h"
 #include "llvm/Support/Errno.h"
 #include <cfenv>
+#if __x86_64__
+#include <xmmintrin.h>
+#endif
 
 namespace Fortran::evaluate::host {
 using namespace Fortran::parser::literals;
 
 void HostFloatingPointEnvironment::SetUpHostFloatingPointEnvironment(
     FoldingContext &context) {
   errno = 0;
+  std::fenv_t currentFenv;
   if (feholdexcept(&originalFenv_) != 0) {
     common::die("Folding with host runtime: feholdexcept() failed: %s",
         llvm::sys::StrError(errno).c_str());
     return;
   }
-  if (fegetenv(&currentFenv_) != 0) {
+  if (fegetenv(&currentFenv) != 0) {
     common::die("Folding with host runtime: fegetenv() failed: %s",
         llvm::sys::StrError(errno).c_str());
     return;
   }
 #if __x86_64__
   hasSubnormalFlushingHardwareControl_ = true;
+  originalMxcsr = _mm_getcsr();
+  unsigned int currentMxcsr{originalMxcsr};
   if (context.flushSubnormalsToZero()) {
-    currentFenv_.__mxcsr |= 0x8000; // result
-    currentFenv_.__mxcsr |= 0x0040; // operands
+    currentMxcsr |= 0x8000;
+    currentMxcsr |= 0x0040;
   } else {
-    currentFenv_.__mxcsr &= ~0x8000; // result
-    currentFenv_.__mxcsr &= ~0x0040; // operands
+    currentMxcsr &= ~0x8000;
+    currentMxcsr &= ~0x0040;
   }
 #elif defined(__aarch64__)
 #if defined(__GNU_LIBRARY__)
   hasSubnormalFlushingHardwareControl_ = true;
   if (context.flushSubnormalsToZero()) {
-    currentFenv_.__fpcr |= (1U << 24); // control register
+    currentFenv.__fpcr |= (1U << 24); // control register
   } else {
-    currentFenv_.__fpcr &= ~(1U << 24); // control register
+    currentFenv.__fpcr &= ~(1U << 24); // control register
   }
 #elif defined(__BIONIC__)
   hasSubnormalFlushingHardwareControl_ = true;
   if (context.flushSubnormalsToZero()) {
-    currentFenv_.__control |= (1U << 24); // control register
+    currentFenv.__control |= (1U << 24); // control register
   } else {
-    currentFenv_.__control &= ~(1U << 24); // control register
+    currentFenv.__control &= ~(1U << 24); // control register
   }
 #else
   // If F18 is built with other C libraries on AArch64, software flushing will
   // be performed around host library calls if subnormal flushing is requested
 #endif
 #else
   // If F18 is not built on one of the above host architecture, software
   // flushing will be performed around host library calls if needed.
 #endif
 
 #ifdef __clang__
   // clang does not ensure that floating point environment flags are meaningful.
   // It may perform optimizations that will impact the floating point
   // environment. For instance, libc++ complex float tan and tanh compilation
   // with clang -O2 introduces a division by zero on X86 in unused slots of xmm
   // registers. Therefore, fetestexcept should not be used.
   hardwareFlagsAreReliable_ = false;
 #endif
   errno = 0;
-  if (fesetenv(&currentFenv_) != 0) {
+  if (fesetenv(&currentFenv) != 0) {
     common::die("Folding with host runtime: fesetenv() failed: %s",
         llvm::sys::StrError(errno).c_str());
     return;
   }
+#if __x86_64__
+  _mm_setcsr(currentMxcsr);
+#endif
+
   switch (context.rounding().mode) {
   case common::RoundingMode::TiesToEven:
     fesetround(FE_TONEAREST);
     break;
   case common::RoundingMode::ToZero:
     fesetround(FE_TOWARDZERO);
     break;
   case common::RoundingMode::Up:
void HostFloatingPointEnvironment::CheckAndRestoreFloatingPointEnvironment(
   errno = 0;
   if (fesetenv(&originalFenv_) != 0) {
     std::fprintf(
         stderr, "fesetenv() failed: %s\n", llvm::sys::StrError(errno).c_str());
     common::die(
         "Folding with host runtime: fesetenv() failed while restoring fenv: %s",
         llvm::sys::StrError(errno).c_str());
   }
+#if __x86_64__
+  _mm_setcsr(originalMxcsr);
+#endif
+
   errno = 0;
 }
 } // namespace Fortran::evaluate::host

flang/unittests/Evaluate/fp-testing.h

ScopedHostFloatingPointEnvironment(bool treatSubnormalOperandsAsZero = false,
       bool flushSubnormalResultsToZero = false);
   ~ScopedHostFloatingPointEnvironment();
   void ClearFlags() const;
   static RealFlags CurrentFlags();
   static void SetRounding(Rounding rounding);
 
 private:
   fenv_t originalFenv_;
-  fenv_t currentFenv_;
+#if __x86_64__
+  unsigned int originalMxcsr;
+#endif
 };
 
 #endif // FORTRAN_TEST_EVALUATE_FP_TESTING_H_

flang/unittests/Evaluate/fp-testing.cpp

 #include "fp-testing.h"
 #include "llvm/Support/Errno.h"
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#if __x86_64__
+#include <xmmintrin.h>
+#endif
 
 using Fortran::common::RoundingMode;
 using Fortran::evaluate::RealFlag;
 
 ScopedHostFloatingPointEnvironment::ScopedHostFloatingPointEnvironment(
 #if __x86_64__
     bool treatSubnormalOperandsAsZero, bool flushSubnormalResultsToZero
 #else
     bool, bool
 #endif
 ) {
   errno = 0;
   if (feholdexcept(&originalFenv_) != 0) {
     std::fprintf(stderr, "feholdexcept() failed: %s\n",
         llvm::sys::StrError(errno).c_str());
     std::abort();
   }
-  if (fegetenv(&currentFenv_) != 0) {
+  fenv_t currentFenv;
+  if (fegetenv(&currentFenv) != 0) {
     std::fprintf(
         stderr, "fegetenv() failed: %s\n", llvm::sys::StrError(errno).c_str());
     std::abort();
   }
+
 #if __x86_64__
+  originalMxcsr = _mm_getcsr();
+  unsigned int currentMxcsr{originalMxcsr};
   if (treatSubnormalOperandsAsZero) {
-    currentFenv_.__mxcsr |= 0x0040;
+    currentMxcsr |= 0x0040;
   } else {
-    currentFenv_.__mxcsr &= ~0x0040;
+    currentMxcsr &= ~0x0040;
   }
   if (flushSubnormalResultsToZero) {
-    currentFenv_.__mxcsr |= 0x8000;
+    currentMxcsr |= 0x8000;
   } else {
-    currentFenv_.__mxcsr &= ~0x8000;
+    currentMxcsr &= ~0x8000;
   }
 #else
   // TODO others
 #endif
   errno = 0;
-  if (fesetenv(&currentFenv_) != 0) {
+  if (fesetenv(&currentFenv) != 0) {
     std::fprintf(
         stderr, "fesetenv() failed: %s\n", llvm::sys::StrError(errno).c_str());
     std::abort();
   }
+#if __x86_64__
+  _mm_setcsr(currentMxcsr);
+#endif
 }
 
 ScopedHostFloatingPointEnvironment::~ScopedHostFloatingPointEnvironment() {
   errno = 0;
   if (fesetenv(&originalFenv_) != 0) {
     std::fprintf(
         stderr, "fesetenv() failed: %s\n", llvm::sys::StrError(errno).c_str());
     std::abort();
   }
+#if __x86_64__
+  _mm_setcsr(originalMxcsr);
+#endif
 }
 
 void ScopedHostFloatingPointEnvironment::ClearFlags() const {
   feclearexcept(FE_ALL_EXCEPT);
 }
 
 RealFlags ScopedHostFloatingPointEnvironment::CurrentFlags() {
   int exceptions = fetestexcept(FE_ALL_EXCEPT);