diff mbox

fix libquadmath build regression

Message ID or7go04ctm.fsf@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva Dec. 30, 2012, 12:13 a.m. UTC
On Dec 24, 2012, Paolo Bonzini <bonzini@gnu.org> wrote:

> Il 21/12/2012 06:17, Alexandre Oliva ha scritto:
>> The problem is that glibc has an extern inline definition of
>> fraiseexcept that is introduced by including fenv.h (it's in
>> bits/fenv.h), and this definition requires SSE support regardless of
>> target arch of word width, so it doesn't work for an i686 native that
>> doesn't assume SSE registers and instructions are available.
>> 
>> This bug is fixed in newer versions of glibc, but I figured it wouldn't
>> hurt to have a work-around in place for libquadmath to build

> Would it be possible to fix it in fixincludes instead?

Heh, who'd have thought of using fixincludes to fix broken include
files? :-)  (hint: not me :-)  *blush*

Thanks for the suggestion, this patch fixes the problem.  Regstrapped on
x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Comments

Paolo Bonzini Dec. 30, 2012, 9:42 a.m. UTC | #1
Il 30/12/2012 01:13, Alexandre Oliva ha scritto:
> On Dec 24, 2012, Paolo Bonzini <bonzini@gnu.org> wrote:
> 
>> Il 21/12/2012 06:17, Alexandre Oliva ha scritto:
>>> The problem is that glibc has an extern inline definition of
>>> fraiseexcept that is introduced by including fenv.h (it's in
>>> bits/fenv.h), and this definition requires SSE support regardless of
>>> target arch of word width, so it doesn't work for an i686 native that
>>> doesn't assume SSE registers and instructions are available.
>>>
>>> This bug is fixed in newer versions of glibc, but I figured it wouldn't
>>> hurt to have a work-around in place for libquadmath to build
> 
>> Would it be possible to fix it in fixincludes instead?
> 
> Heh, who'd have thought of using fixincludes to fix broken include
> files? :-)  (hint: not me :-)  *blush*
> 
> Thanks for the suggestion, this patch fixes the problem.  Regstrapped on
> x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Not my territory anymore, but it looks much better!  CCing Bruce.

Paolo
Bruce Korb Dec. 30, 2012, 10:26 p.m. UTC | #2
On 12/30/12 01:42, Paolo Bonzini wrote:
> Not my territory anymore, but it looks much better!  CCing Bruce.

Hi Alexandre,

Long time.  It's no wonder you've forgotten this little world! :)

Anyway, please make the expressions more readable and strip
out the generated text from the review message.

Readability guidance:  indent text and break lines logically.
Consider "here-strings".  e.g. not:

    c_fix_arg = "# ifdef __SSE_MATH__\n%0\n"
    "# else\n%1__asm__ __volatile__ (\"fdiv %%%%st, %%%%st(0); fwait\"\n"
    "%1\t\t\t: \"=t\" (__f) : \"0\" (__f));\n"
    "# endif";

instead:

    c_fix_arg = "# ifdef __SSE_MATH__\n%0\n"
                "# else\n"
                "%1__asm__ __volatile__ (\"fdiv %%%%st, %%%%st(0); fwait\"\n"
                "%1\t\t\t: \"=t\" (__f) : \"0\" (__f));\n"
                "# endif";

or even better:

    c_fix_arg = <<- _EOText_
	# ifdef __SSE_MATH__\n
	%0
	# else
	%1__asm__ __volatile__ ("fdiv %%%%st, %%%%st(0); fwait"
	%1			"=t" (__f) : "0" (__f));
	# endif
	_EOText_;
diff mbox

Patch

Fix mandatory SSE in 2.1[56]ish glibc's feraiseexcept

From: Alexandre Oliva <aoliva@redhat.com>

for fixincludes/ChangeLog

	* inclhack.def (feraiseexcept_nosse_invalid): New.
	(feraiseexcept_nosse_divbyzero): Likewise.
	* fixincl.x: Rebuilt.
	* tests/base/bits/fenv.h: New.
---

 fixincludes/fixincl.x              |  130 ++++++++++++++++++++++++++++++++++--
 fixincludes/inclhack.def           |   39 +++++++++++
 fixincludes/tests/base/bits/fenv.h |   29 ++++++++
 3 files changed, 190 insertions(+), 8 deletions(-)
 create mode 100644 fixincludes/tests/base/bits/fenv.h


diff --git a/fixincludes/fixincl.x b/fixincludes/fixincl.x
index 4115772..29fc52a 100644
--- a/fixincludes/fixincl.x
+++ b/fixincludes/fixincl.x
@@ -2,11 +2,11 @@ 
  * 
  * DO NOT EDIT THIS FILE   (fixincl.x)
  * 
- * It has been AutoGen-ed  October 28, 2012 at 02:53:25 PM by AutoGen 5.17.0pre5
+ * It has been AutoGen-ed  Saturday December 29, 2012 at 09:17:09 AM BRST
  * From the definitions    inclhack.def
  * and the template file   fixincl
  */
-/* DO NOT SVN-MERGE THIS FILE, EITHER Sun Oct 28 14:53:25 PDT 2012
+/* DO NOT SVN-MERGE THIS FILE, EITHER Sat Dec 29 09:17:10 BRST 2012
  *
  * You must regenerate it.  Use the ./genfixes script.
  *
@@ -15,7 +15,7 @@ 
  * certain ANSI-incompatible system header files which are fixed to work
  * correctly with ANSI C and placed in a directory that GNU C will search.
  *
- * This file contains 224 fixup descriptions.
+ * This file contains 226 fixup descriptions.
  *
  * See README for more information.
  *
@@ -9158,14 +9158,116 @@  static const char* apzX11_SprintfPatch[] = {
 #endif /* !defined __STDC__ */",
     (char*)NULL };
 
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ *
+ *  Description of Feraiseexcept_Nosse_Invalid fix
+ */
+tSCC zFeraiseexcept_Nosse_InvalidName[] =
+     "feraiseexcept_nosse_invalid";
+
+/*
+ *  File name selection pattern
+ */
+tSCC zFeraiseexcept_Nosse_InvalidList[] =
+  "bits/fenv.h\0";
+/*
+ *  Machine/OS name selection pattern
+ */
+tSCC* apzFeraiseexcept_Nosse_InvalidMachs[] = {
+        "i[34567]86-*-linux*",
+        "x86*-linux*",
+        "amd64-*-linux*",
+        (const char*)NULL };
+
+/*
+ *  content selection pattern - do fix if pattern found
+ */
+tSCC zFeraiseexcept_Nosse_InvalidSelect0[] =
+       "^([\t ]*)__asm__ __volatile__ \\(\"divss %0, %0 *\" : : \"x\" \\(__f\\)\\);$";
+
+/*
+ *  content bypass pattern - skip fix if pattern found
+ */
+tSCC zFeraiseexcept_Nosse_InvalidBypass0[] =
+       "\"fdiv .*; fwait\"";
+
+#define    FERAISEEXCEPT_NOSSE_INVALID_TEST_CT  2
+static tTestDesc aFeraiseexcept_Nosse_InvalidTests[] = {
+  { TT_NEGREP,   zFeraiseexcept_Nosse_InvalidBypass0, (regex_t*)NULL },
+  { TT_EGREP,    zFeraiseexcept_Nosse_InvalidSelect0, (regex_t*)NULL }, };
+
+/*
+ *  Fix Command Arguments for Feraiseexcept_Nosse_Invalid
+ */
+static const char* apzFeraiseexcept_Nosse_InvalidPatch[] = {
+    "format",
+    "# ifdef __SSE_MATH__\n\
+%0\n\
+# else\n\
+%1__asm__ __volatile__ (\"fdiv %%%%st, %%%%st(0); fwait\"\n\
+%1\t\t\t: \"=t\" (__f) : \"0\" (__f));\n\
+# endif",
+    (char*)NULL };
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ *
+ *  Description of Feraiseexcept_Nosse_Divbyzero fix
+ */
+tSCC zFeraiseexcept_Nosse_DivbyzeroName[] =
+     "feraiseexcept_nosse_divbyzero";
+
+/*
+ *  File name selection pattern
+ */
+tSCC zFeraiseexcept_Nosse_DivbyzeroList[] =
+  "bits/fenv.h\0";
+/*
+ *  Machine/OS name selection pattern
+ */
+tSCC* apzFeraiseexcept_Nosse_DivbyzeroMachs[] = {
+        "i[34567]86-*-linux*",
+        "x86*-linux*",
+        "amd64-*-linux*",
+        (const char*)NULL };
+
+/*
+ *  content selection pattern - do fix if pattern found
+ */
+tSCC zFeraiseexcept_Nosse_DivbyzeroSelect0[] =
+       "^([\t ]*)__asm__ __volatile__ \\(\"divss %1, %0 *\" : : \"x\" \\(__f\\), \"x\" \\(__g\\)\\);$";
+
+/*
+ *  content bypass pattern - skip fix if pattern found
+ */
+tSCC zFeraiseexcept_Nosse_DivbyzeroBypass0[] =
+       "\"fdivp .*; fwait\"";
+
+#define    FERAISEEXCEPT_NOSSE_DIVBYZERO_TEST_CT  2
+static tTestDesc aFeraiseexcept_Nosse_DivbyzeroTests[] = {
+  { TT_NEGREP,   zFeraiseexcept_Nosse_DivbyzeroBypass0, (regex_t*)NULL },
+  { TT_EGREP,    zFeraiseexcept_Nosse_DivbyzeroSelect0, (regex_t*)NULL }, };
+
+/*
+ *  Fix Command Arguments for Feraiseexcept_Nosse_Divbyzero
+ */
+static const char* apzFeraiseexcept_Nosse_DivbyzeroPatch[] = {
+    "format",
+    "# ifdef __SSE_MATH__\n\
+%0\n\
+# else\n\
+%1__asm__ __volatile__ (\"fdivp %%%%st, %%%%st(1); fwait\"\n\
+%1\t\t\t: \"=t\" (__f) : \"0\" (__f), \"u\" (__g) : \"st(1)\");\n\
+# endif",
+    (char*)NULL };
+
 
 /* * * * * * * * * * * * * * * * * * * * * * * * * *
  *
  *  List of all fixes
  */
-#define REGEX_COUNT          260
-#define MACH_LIST_SIZE_LIMIT 181
-#define FIX_COUNT            224
+#define REGEX_COUNT          264
+#define MACH_LIST_SIZE_LIMIT 187
+#define FIX_COUNT            226
 
 /*
  *  Enumerate the fixes
@@ -9394,7 +9496,9 @@  typedef enum {
     X11_CLASS_FIXIDX,
     X11_CLASS_USAGE_FIXIDX,
     X11_NEW_FIXIDX,
-    X11_SPRINTF_FIXIDX
+    X11_SPRINTF_FIXIDX,
+    FERAISEEXCEPT_NOSSE_INVALID_FIXIDX,
+    FERAISEEXCEPT_NOSSE_DIVBYZERO_FIXIDX
 } t_fixinc_idx;
 
 tFixDesc fixDescList[ FIX_COUNT ] = {
@@ -10516,5 +10620,15 @@  tFixDesc fixDescList[ FIX_COUNT ] = {
   {  zX11_SprintfName,    zX11_SprintfList,
      apzX11_SprintfMachs,
      X11_SPRINTF_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
-     aX11_SprintfTests,   apzX11_SprintfPatch, 0 }
+     aX11_SprintfTests,   apzX11_SprintfPatch, 0 },
+
+  {  zFeraiseexcept_Nosse_InvalidName,    zFeraiseexcept_Nosse_InvalidList,
+     apzFeraiseexcept_Nosse_InvalidMachs,
+     FERAISEEXCEPT_NOSSE_INVALID_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
+     aFeraiseexcept_Nosse_InvalidTests,   apzFeraiseexcept_Nosse_InvalidPatch, 0 },
+
+  {  zFeraiseexcept_Nosse_DivbyzeroName,    zFeraiseexcept_Nosse_DivbyzeroList,
+     apzFeraiseexcept_Nosse_DivbyzeroMachs,
+     FERAISEEXCEPT_NOSSE_DIVBYZERO_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
+     aFeraiseexcept_Nosse_DivbyzeroTests,   apzFeraiseexcept_Nosse_DivbyzeroPatch, 0 }
 };
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 09eac7c6..b06e370 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -4815,4 +4815,43 @@  fix = {
     test_text = "extern char *\tsprintf();";
 };
 
+/*
+ *  Incorrect feraiseexcept extern inline in bits/fenv.h on x86_64
+ *  that fails when compiling for SSE-less 32-bit x86.
+ */
+fix = {
+    hackname = feraiseexcept_nosse_invalid;
+    mach     = 'i[34567]86-*-linux*', 'x86*-linux*', 'amd64-*-linux*';
+    files    = bits/fenv.h;
+    select   = "^([\t ]*)__asm__ __volatile__ \\(\"divss %0, %0 *\" : "
+    ": \"x\" \\(__f\\)\\);$";
+    bypass   = "\"fdiv .*; fwait\"";
+    
+    c_fix     = format;
+    c_fix_arg = "# ifdef __SSE_MATH__\n%0\n"
+    "# else\n%1__asm__ __volatile__ (\"fdiv %%%%st, %%%%st(0); fwait\"\n"
+    "%1\t\t\t: \"=t\" (__f) : \"0\" (__f));\n"
+    "# endif";
+
+    test_text = "\t __asm__ __volatile__ (\"divss %0, %0\" : "
+    ": \"x\" (__f));";
+};
+fix = {
+    hackname = feraiseexcept_nosse_divbyzero;
+    mach     = 'i[34567]86-*-linux*', 'x86*-linux*', 'amd64-*-linux*';
+    files    = bits/fenv.h;
+    select   = "^([\t ]*)__asm__ __volatile__ \\(\"divss %1, %0 *\" : "
+    ": \"x\" \\(__f\\), \"x\" \\(__g\\)\\);$";
+    bypass   = "\"fdivp .*; fwait\"";
+    
+    c_fix     = format;
+    c_fix_arg = "# ifdef __SSE_MATH__\n%0\n"
+    "# else\n%1__asm__ __volatile__ (\"fdivp %%%%st, %%%%st(1); fwait\"\n"
+    "%1\t\t\t: \"=t\" (__f) : \"0\" (__f), \"u\" (__g) : \"st(1)\");\n"
+    "# endif";
+
+    test_text = "\t __asm__ __volatile__ (\"divss %1, %0\" : "
+    ": \"x\" (__f), \"x\" (__g));";
+};
+
 /*EOF*/
diff --git a/fixincludes/tests/base/bits/fenv.h b/fixincludes/tests/base/bits/fenv.h
new file mode 100644
index 0000000..4b8b71a
--- /dev/null
+++ b/fixincludes/tests/base/bits/fenv.h
@@ -0,0 +1,29 @@ 
+/*  DO NOT EDIT THIS FILE.
+
+    It has been auto-edited by fixincludes from:
+
+	"fixinc/tests/inc/bits/fenv.h"
+
+    This had to be done to correct non-standard usages in the
+    original, manufacturer supplied header file.  */
+
+
+
+#if defined( FERAISEEXCEPT_NOSSE_INVALID_CHECK )
+# ifdef __SSE_MATH__
+	 __asm__ __volatile__ ("divss %0, %0" : : "x" (__f));
+# else
+	 __asm__ __volatile__ ("fdiv %%st, %%st(0); fwait"
+	 			: "=t" (__f) : "0" (__f));
+# endif
+#endif  /* FERAISEEXCEPT_NOSSE_INVALID_CHECK */
+
+
+#if defined( FERAISEEXCEPT_NOSSE_DIVBYZERO_CHECK )
+# ifdef __SSE_MATH__
+	 __asm__ __volatile__ ("divss %1, %0" : : "x" (__f), "x" (__g));
+# else
+	 __asm__ __volatile__ ("fdivp %%st, %%st(1); fwait"
+	 			: "=t" (__f) : "0" (__f), "u" (__g) : "st(1)");
+# endif
+#endif  /* FERAISEEXCEPT_NOSSE_DIVBYZERO_CHECK */