diff mbox

i686: Add missing IS_IN (libc) guards to vectorized strcspn

Message ID 20170614061950.400FE4010728F@oldenburg.str.redhat.com
State New
Headers show

Commit Message

Florian Weimer June 14, 2017, 6:19 a.m. UTC
Since commit d957c4d3fa48d685ff2726c605c988127ef99395 (i386: Compile
rtld-*.os with -mno-sse -mno-mmx -mfpmath=387), vector intrinsics can
no longer be used in ld.so, even if the compiled code never makes it
into the final ld.so link.  This commit adds the missing IS_IN (libc)
guard to the SSE 4.2 strcspn implementation, so that it can be used from
ld.so in the future.

2017-06-14  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/i386/i686/multiarch/strcspn-c.c: Add IS_IN (libc) guard.
	* sysdeps/i386/i686/multiarch/varshift.c: Likewise.

Comments

Carlos O'Donell June 14, 2017, 12:32 p.m. UTC | #1
On 06/14/2017 02:19 AM, Florian Weimer wrote:
> Since commit d957c4d3fa48d685ff2726c605c988127ef99395 (i386: Compile
> rtld-*.os with -mno-sse -mno-mmx -mfpmath=387), vector intrinsics can
> no longer be used in ld.so, even if the compiled code never makes it
> into the final ld.so link.  This commit adds the missing IS_IN (libc)
> guard to the SSE 4.2 strcspn implementation, so that it can be used from
> ld.so in the future.
> 
> 2017-06-14  Florian Weimer  <fweimer@redhat.com>
> 
> 	* sysdeps/i386/i686/multiarch/strcspn-c.c: Add IS_IN (libc) guard.
> 	* sysdeps/i386/i686/multiarch/varshift.c: Likewise.

This looks good to me.

We have struggled with these issues for years. I wish there was a more
automatic way to enable new functions for use in ld.so, but there isn't.

Out of curiosity, if you don't apply this patch, and use strcspn, does
the tst-ld-sse-use.sh test fail?
 
> diff --git a/sysdeps/i386/i686/multiarch/strcspn-c.c b/sysdeps/i386/i686/multiarch/strcspn-c.c
> index 6d61e19..ec230fb 100644
> --- a/sysdeps/i386/i686/multiarch/strcspn-c.c
> +++ b/sysdeps/i386/i686/multiarch/strcspn-c.c
> @@ -1,2 +1,4 @@
> -#define __strcspn_sse2 __strcspn_ia32
> -#include <sysdeps/x86_64/multiarch/strcspn-c.c>
> +#if IS_IN (libc)
> +# define __strcspn_sse2 __strcspn_ia32
> +# include <sysdeps/x86_64/multiarch/strcspn-c.c>
> +#endif
> diff --git a/sysdeps/i386/i686/multiarch/varshift.c b/sysdeps/i386/i686/multiarch/varshift.c
> index 7760b96..6742a35 100644
> --- a/sysdeps/i386/i686/multiarch/varshift.c
> +++ b/sysdeps/i386/i686/multiarch/varshift.c
> @@ -1 +1,3 @@
> -#include <sysdeps/x86_64/multiarch/varshift.c>
> +#if IS_IN (libc)
> +# include <sysdeps/x86_64/multiarch/varshift.c>
> +#endif
>
Florian Weimer June 14, 2017, 12:40 p.m. UTC | #2
On 06/14/2017 02:32 PM, Carlos O'Donell wrote:
> On 06/14/2017 02:19 AM, Florian Weimer wrote:
>> Since commit d957c4d3fa48d685ff2726c605c988127ef99395 (i386: Compile
>> rtld-*.os with -mno-sse -mno-mmx -mfpmath=387), vector intrinsics can
>> no longer be used in ld.so, even if the compiled code never makes it
>> into the final ld.so link.  This commit adds the missing IS_IN (libc)
>> guard to the SSE 4.2 strcspn implementation, so that it can be used from
>> ld.so in the future.
>>
>> 2017-06-14  Florian Weimer  <fweimer@redhat.com>
>>
>> 	* sysdeps/i386/i686/multiarch/strcspn-c.c: Add IS_IN (libc) guard.
>> 	* sysdeps/i386/i686/multiarch/varshift.c: Likewise.
> 
> This looks good to me.

Thanks.

> We have struggled with these issues for years. I wish there was a more
> automatic way to enable new functions for use in ld.so, but there isn't.
> 
> Out of curiosity, if you don't apply this patch, and use strcspn, does
> the tst-ld-sse-use.sh test fail?

No.  The files containing IFUNC resolvers are compiled in rtld mode
(!IS_IN (libc)), and they do not reference the definitions in
strcpsn-c.c anymore.  But the list of objects is constructed based on
the contents of libc-pic.a, and that obviously contains strcspn-c.o.
With current upstream, after adding a reference to strcspn within ld.so,
we attempt to recompile strcspn-c.c, but that fails with GCC errors due
to -mno-sse etc. added as a safeguard.  Prior to commit
d957c4d3fa48d685ff2726c605c988127ef99395, strcspn-c.c would compile
successfully, but since the IFUNC resolver is compiled out due to !IS_IN
(libc), the link editor will not pick up strcspn-c.o, although it is
present in rtld-libc.a.

Florian
diff mbox

Patch

diff --git a/sysdeps/i386/i686/multiarch/strcspn-c.c b/sysdeps/i386/i686/multiarch/strcspn-c.c
index 6d61e19..ec230fb 100644
--- a/sysdeps/i386/i686/multiarch/strcspn-c.c
+++ b/sysdeps/i386/i686/multiarch/strcspn-c.c
@@ -1,2 +1,4 @@ 
-#define __strcspn_sse2 __strcspn_ia32
-#include <sysdeps/x86_64/multiarch/strcspn-c.c>
+#if IS_IN (libc)
+# define __strcspn_sse2 __strcspn_ia32
+# include <sysdeps/x86_64/multiarch/strcspn-c.c>
+#endif
diff --git a/sysdeps/i386/i686/multiarch/varshift.c b/sysdeps/i386/i686/multiarch/varshift.c
index 7760b96..6742a35 100644
--- a/sysdeps/i386/i686/multiarch/varshift.c
+++ b/sysdeps/i386/i686/multiarch/varshift.c
@@ -1 +1,3 @@ 
-#include <sysdeps/x86_64/multiarch/varshift.c>
+#if IS_IN (libc)
+# include <sysdeps/x86_64/multiarch/varshift.c>
+#endif