Re: [lmi] MinGW-w64 anomaly?

[Vadim Zeitlin]

On Mon, 26 Dec 2016 08:37:22 +0000 Greg Chicares <address@hidden> wrote:

GC> For the time being, does the following patch work on x86_64, and if so,
GC> should I apply it?

 This patch does try to do the right thing, but fails because of 2 things:
first, it doesn't compile due to duplicate e_ieee754_rounding declaration
and, second, just removing as advised by the comment isn't enough because
we still pass the wrong values to fesetround() (which took me some time to
find out -- isn't it great to have untyped interfaces in the C++ standard
library...).

 So here is the fixed version which does make the test pass:
---------------------------------- >8 --------------------------------------
diff --git a/round_to_test.cpp b/round_to_test.cpp
index e013709..8b1d4c6 100644
--- a/round_to_test.cpp
+++ b/round_to_test.cpp
@@ -33,6 +33,13 @@
 #include <ostream>

 #if defined LMI_IEC_559
+    // The 'LMI_IEC_559' macro is never defined by any source file or
+    // by any makefile, so this conditional block is not reached today.
+    // For the macro's original rationale, see:
+    //   http://lists.nongnu.org/archive/html/lmi/2008-06/msg00034.html
+    // The description that follows is outdated anyway, because C++11
+    // now provides <cfenv>.
+    //
     // In case the C++ compiler offers C99 fesetround(), assume that
     // it defines __STDC_IEC_559__, but doesn't support
     //   #pragma STDC FENV_ACCESS ON
@@ -44,7 +51,17 @@
         ,fe_upward     = FE_UPWARD
         ,fe_towardzero = FE_TOWARDZERO
         };
+#elif defined LMI_X86_64
+    // Probably this conditional should distinguish SSE from x87,
+    // rather than 64- from 32-bit x64. For the nonce, this prevents
+    // certain unit-test failures noted here:
+    //   http://lists.nongnu.org/archive/html/lmi/2016-12/msg00053.html
+#   include <cfenv>
 #elif defined LMI_X86
+    // It seems at first that the conditional here should be LMI_X86_32
+    // because x86_64 is treated above. However, perhaps the actual
+    // distinction is between SSE above and x87 here.
+    //
     // "fenv_lmi_x86.hpp" provides the necessary values.
 #else  // No known way to set rounding style.
 #   error No known way to set rounding style.
@@ -102,6 +119,16 @@ void set_hardware_rounding_mode(e_ieee754_rounding mode, 
bool synchronize)
 {
 #if defined LMI_IEC_559
     fesetround(mode);
+#elif defined LMI_X86_64
+    // See comments above on the <cfenv> series of conditionals.
+    // For the nonce, set both i87 and SSE rounding modes here.
+    fesetround(  (fe_tonearest  == mode) ? FE_TONEAREST
+               : (fe_downward   == mode) ? FE_DOWNWARD
+               : (fe_upward     == mode) ? FE_UPWARD
+               : (fe_towardzero == mode) ? FE_TOWARDZERO
+               : throw std::runtime_error("Failed to set rounding mode.")
+              );
+    fenv_rounding(mode);
 #elif defined LMI_X86
     fenv_rounding(mode);
 #else // No known way to set hardware rounding mode.
---------------------------------- >8 --------------------------------------

 Of course, this is not the final version if only because it doesn't make
much sense to reproduce the code which is already present (but inside
LMI_IEC_559 check) in fend_rounding() itself, so I'd rather change it
instead.

 But I'm still not quite sure whether I should work on the patch doing
what, I think, we've now agreed to do or if I should leave it to you?
Again, my idea is to reuse the existing LMI_IEC_559 code and always enable
it for platforms not using x87, i.e. i386 code if the symbol indicating
the use of SSE is not defined, and all the other platforms.

 Should I do it, test it and submit it as a proper patch/pull request?
VZ