Re: [lmi] [PATCH] Better support for non-x87 platforms

[Vadim Zeitlin]

On Thu, 5 Jan 2017 03:41:12 +0000 Greg Chicares <address@hidden> wrote:

GC> [See commit 571924bbc8f29c3002e28e4ee3f1c3f2c3269d22.]

 Sorry, but this still doesn't compile under non-x87 platforms because of
the

#       error Unknown compiler--cannot detect SSE. Consider contributing 
support.

line in config.hpp which is triggered in this case.

 Looking at the diff between my original changes and current master:
---------------------------------- >8 --------------------------------------
diff --git a/config.hpp b/config.hpp
index 7d61eca..7f0557b 100644
--- a/config.hpp
+++ b/config.hpp
@@ -68,17 +68,6 @@ namespace fs = boost::filesystem;
 #   error Unknown hardware. Consider contributing support.
 #endif // Unknown hardware.

-#if defined LMI_X86
-    #if defined _M_IX86_FP && _M_IX86_FP == 0
-        #define LMI_X87
-    // Test for __SSE_MATH__ instead of __SSE__ as the latter is still defined
-    // in x86-64 builds even if the use of SSE is explicitly disabled using gcc
-    // -mfpmath=387 option.
-    #elif !defined __SSE_MATH__
-        #define LMI_X87
-    #endif
-#endif // defined LMI_X86
-
 #if defined __GNUC__
 // This selects a correct snprintf() for MinGW-w64.
 #   if !defined _ISOC99_SOURCE
@@ -159,6 +148,22 @@ namespace fs = boost::filesystem;
 #   define LMI_MSVCRT
 #endif // Compilers that use the msvc C runtime, without corrections such as 
libmingwex.

+// Test for x87.
+//
+// For gcc, test the __SSE_MATH__ macro. It might seem logical to test
+// the __SSE__ macro instead; however, for x86_64, gcc defines __SSE__
+// even if SSE is explicitly disabled with '-mfpmath=387'.
+
+#if defined LMI_X86
+#   if defined __GNUC__ && !defined __SSE_MATH__
+#       define LMI_X87
+#   elif defined LMI_MSC && defined _M_IX86_FP && _M_IX86_FP == 0
+#       define LMI_X87
+#   else   // Unknown compiler.
+#       error Unknown compiler--cannot detect SSE. Consider contributing 
support.
+#   endif  // Unknown compiler.
+#endif // defined LMI_X86
+
 #if defined HAVE_CONFIG_H // Using autoconf.
 #   include "config.h"
 #else // Not using autoconf.
---------------------------------- >8 --------------------------------------

I'm not sure why was this changed. Moving these checks (much) further down
doesn't look really good to me neither because IMHO it would make sense to
define LMI_X87 as near as possible from the place where LMI_X86 is defined
because both of them will likely need to be updated together if support for
a new platform is added but the main problem is, of course, this "#error"
line which, IMO, should be simply removed. Alternatively, if we really want
to keep it, we should rewrite the conditions as

        #if defined __GNUC__
                #if !defined __SSE_MATH__
                        #define LMI_X87
                #endif
        #elif defined LMI_MSC
                #if defined _M_IX86_FP && _M_IX86_FP == 0
                        #define LMI_X87
                #endif
        #else
                #error Unknown compiler.
        #endif

 Regards,
VZ