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

[Andreas Jaeger <aj at suse dot com>]

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?

> 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.

> 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.

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

thanks,
Andreas
--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126