Re: PATCH: Compile x86 rtld with -mno-sse -mno-mmx

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Nov 02, 2012 at 11:24:09AM +0100, Andreas Jaeger wrote:
> On 11/02/2012 10:41 AM, H.J. Lu wrote:
> >Hi,
> >
> >This patch compiles x86 rtld with -mno-sse -mno-mmx.  It introduces
> >bits/stdlib-float.h so that we can avoid inlined atof in rtld.  It
> 
> Is that just caution or is there a real usage of atof?

Since it is an inline fuction, GCC issues an error when -mno-sse is
used.

> 
> >also avoids <xmmintrin.h> in rtld.  We only need a 128-bit type
> >in sysdeps/x86_64/tls.h since we use it with explicit alignment
> >attribute.  strcasestr-nonascii.c uses __128mi without including
> ><xmmintrin.h>.  This patch fixes it.  -mno-sse -mno-mmx applie
> >to both ia32 and x86-64 rtld since GCC may use SSE registers even
> >in 32-bit, especially with LRA in GCC 4.8 which uses SSE registers
> >for GPR spill in 32-bit.
> >
> >Tested on ia32 and x86-64.  OK to install?
> >
> >Thanks.
> >
> >
> >H.J.
> >---
> >2012-11-02  H.J. Lu  <hongjiu.lu@intel.com>
> >
> >	* stdlib/Makefile (headers): Add bits/stdlib-float.h.
> >	* stdlib/stdlib.h (atof): Moved to ...
> >	* include/bits/stdlib-float.h: Here.  New file.
> >	* stdlib/stdlib.h: Include <bits/stdlib-float.h>.
> >	* stdlib/bits/stdlib-float.h: New file.
> >	* sysdeps/x86/Makefile (CFLAGS-.os): Compile rtld routines with
> >	-mno-sse -mno-mmx.
> >	* sysdeps/x86_64/multiarch/strcasestr-nonascii.c: Include
> >	<xmmintrin.h>.
> >
> >nptl/
> >
> >2012-11-02  H.J. Lu  <hongjiu.lu@intel.com>
> >
> >	* sysdeps/x86_64/tls.h: Don't include <xmmintrin.h>.
> >	(__128bits): New struct typedef.
> >	(tcbhead_t): Replace __m128 with __128bits.
> >
> >diff --git a/include/bits/stdlib-float.h b/include/bits/stdlib-float.h
> >new file mode 100644
> >index 0000000..d3f5c5b
> >--- /dev/null
> >+++ b/include/bits/stdlib-float.h
> >@@ -0,0 +1,4 @@
> >+/* No floating-point inline functions in rtld.  */
> >+#ifndef IS_IN_rtld
> >+# include <stdlib/bits/stdlib-float.h>
> >+#endif
> 
> I suggest to use #include_next <bits/stdlib-float.h> here, an
> architecture might want to override the file.

<stdlib.h> can't be overrided by architecture since it is placed
under stdlib and -I../stdlib isn't used to compile glibc. We use

#include <stdlib/stdlib.h>

in include/stdlib.h.

> 
> >diff --git a/nptl/sysdeps/x86_64/tls.h b/nptl/sysdeps/x86_64/tls.h
> >index b651d1c..4dee137 100644
> >--- a/nptl/sysdeps/x86_64/tls.h
> >+++ b/nptl/sysdeps/x86_64/tls.h
> >@@ -27,7 +27,10 @@
> >  # include <stdlib.h>
> >  # include <sysdep.h>
> >  # include <kernel-features.h>
> >-# include <xmmintrin.h>
> >+typedef struct
> >+{
> >+  int i[4];
> >+} __128bits;
> >
> >
> >  /* Type for the dtv.  */
> >@@ -64,7 +67,7 @@ typedef struct
> >    void *__private_tm[5];
> >    long int __unused2;
> >    /* Have space for the post-AVX register size.  */
> >-  __m128 rtld_savespace_sse[8][4] __attribute__ ((aligned (32)));
> >+  __128bits rtld_savespace_sse[8][4] __attribute__ ((aligned (32)));
> 
> I suggest to add a comment above why __m128 is not used here.

Done.

> 
> I'd like somebody else to review this as well,
> 

I also fixed the typo pointed out by Chris.  OK to install?

Thanks.


H.J.
---
2012-11-02  H.J. Lu  <hongjiu.lu@intel.com>

	* stdlib/Makefile (headers): Add bits/stdlib-float.h.
	* stdlib/stdlib.h (atof): Moved to ...
	* include/bits/stdlib-float.h: Here.  New file.
	* stdlib/stdlib.h: Include <bits/stdlib-float.h>.
	* stdlib/bits/stdlib-float.h: New file.
	* sysdeps/x86/Makefile (CFLAGS-.os): Compile rtld routines with
	-mno-sse -mno-mmx.
	* sysdeps/x86_64/multiarch/strcasestr-nonascii.c: Include
	<xmmintrin.h>.

nptl/

2012-11-02  H.J. Lu  <hongjiu.lu@intel.com>

	* sysdeps/x86_64/tls.h: Don't include <xmmintrin.h>.
	(__128bits): New struct typedef.
	(tcbhead_t): Replace __m128 with __128bits.

diff --git a/include/bits/stdlib-float.h b/include/bits/stdlib-float.h
new file mode 100644
index 0000000..d3f5c5b
--- /dev/null
+++ b/include/bits/stdlib-float.h
@@ -0,0 +1,4 @@
+/* No floating-point inline functions in rtld.  */
+#ifndef IS_IN_rtld
+# include <stdlib/bits/stdlib-float.h>
+#endif
diff --git a/nptl/sysdeps/x86_64/tls.h b/nptl/sysdeps/x86_64/tls.h
index b651d1c..bc60a51 100644
--- a/nptl/sysdeps/x86_64/tls.h
+++ b/nptl/sysdeps/x86_64/tls.h
@@ -27,7 +27,14 @@
 # include <stdlib.h>
 # include <sysdep.h>
 # include <kernel-features.h>
-# include <xmmintrin.h>
+
+/* Replacement type for __m128 since this file is included by ld.so,
+   which is compiled with -mno-sse.  It must not change the alignment
+   of rtld_savespace_sse.  */
+typedef struct
+{
+  int i[4];
+} __128bits;
 
 
 /* Type for the dtv.  */
@@ -64,7 +71,7 @@ typedef struct
   void *__private_tm[5];
   long int __unused2;
   /* Have space for the post-AVX register size.  */
-  __m128 rtld_savespace_sse[8][4] __attribute__ ((aligned (32)));
+  __128bits rtld_savespace_sse[8][4] __attribute__ ((aligned (32)));
 
   void *__padding[8];
 } tcbhead_t;
diff --git a/stdlib/Makefile b/stdlib/Makefile
index 57830a8..5e99d7f 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -20,7 +20,7 @@
 #
 subdir	:= stdlib
 
-headers	:= stdlib.h bits/stdlib.h bits/stdlib-ldbl.h			      \
+headers	:= stdlib.h bits/stdlib.h bits/stdlib-ldbl.h bits/stdlib-float.h      \
 	   monetary.h bits/monetary-ldbl.h				      \
 	   inttypes.h stdint.h bits/wordsize.h				      \
 	   errno.h sys/errno.h bits/errno.h				      \
diff --git a/stdlib/bits/stdlib-float.h b/stdlib/bits/stdlib-float.h
new file mode 100644
index 0000000..46f3a32
--- /dev/null
+++ b/stdlib/bits/stdlib-float.h
@@ -0,0 +1,31 @@
+/* Floating-point inline functions for stdlib.h.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _STDLIB_H
+# error "Never use <bits/stdlib-float.h> directly; include <stdlib.h> instead."
+#endif
+
+#ifdef __USE_EXTERN_INLINES
+__BEGIN_NAMESPACE_STD
+__extern_inline double
+__NTH (atof (const char *__nptr))
+{
+  return strtod (__nptr, (char **) NULL);
+}
+__END_NAMESPACE_STD
+#endif /* Optimizing and Inlining.  */
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index cf3f39c..0eb982c 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -274,11 +274,6 @@ extern long double strtold_l (const char *__restrict __nptr,
 
 #ifdef __USE_EXTERN_INLINES
 __BEGIN_NAMESPACE_STD
-__extern_inline double
-__NTH (atof (const char *__nptr))
-{
-  return strtod (__nptr, (char **) NULL);
-}
 __extern_inline int
 __NTH (atoi (const char *__nptr))
 {
@@ -953,6 +948,7 @@ extern int getloadavg (double __loadavg[], int __nelem)
      __THROW __nonnull ((1));
 #endif
 
+#include <bits/stdlib-float.h>
 
 /* Define some macros helping to catch buffer overflows.  */
 #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 7589b25..c024729 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -1,4 +1,6 @@
 ifeq ($(subdir),elf)
+CFLAGS-.os += $(if $(filter $(@F),$(patsubst %,%.os,$(all-rtld-routines))),\
+		   -mno-sse -mno-mmx)
 tests: $(objpfx)tst-xmmymm.out
 $(objpfx)tst-xmmymm.out: ../sysdeps/x86/tst-xmmymm.sh $(objpfx)ld.so
 	@echo "Checking ld.so for SSE register use.  This will take a few seconds..."
diff --git a/sysdeps/x86_64/multiarch/strcasestr-nonascii.c b/sysdeps/x86_64/multiarch/strcasestr-nonascii.c
index a1f9968..db84a43 100644
--- a/sysdeps/x86_64/multiarch/strcasestr-nonascii.c
+++ b/sysdeps/x86_64/multiarch/strcasestr-nonascii.c
@@ -16,7 +16,8 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-# include <ctype.h>
+#include <ctype.h>
+#include <xmmintrin.h>
 
 
 /* Similar to __m128i_strloadu.  Convert to lower case for none-POSIX/C