This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] BZ #19590: Fixed build of shared objects that use libmvec.so functions
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Andrew Senkevich <andrew dot n dot senkevich at gmail dot com>
- Cc: Joseph Myers <joseph at codesourcery dot com>, libc-alpha <libc-alpha at sourceware dot org>
- Date: Tue, 16 Feb 2016 05:09:34 -0800
- Subject: Re: [PATCH] BZ #19590: Fixed build of shared objects that use libmvec.so functions
- Authentication-results: sourceware.org; auth=none
- References: <CAMXFM3tML81iuKQMKRU-T4Fw0+=sYk0q_BNavMGagt21VcYvzQ at mail dot gmail dot com> <CAMe9rOp7BF2avdWbGTbsxuYQV_rmXSxzDUAwz+nHK-GhWqPKJA at mail dot gmail dot com> <CAMXFM3sG90jn3Em-REfaqtj3OGAjh51OcO=yn1qHqJ4aStkPrg at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1602111641250 dot 29940 at digraph dot polyomino dot org dot uk> <CAMXFM3uxH=0DHnwikPjs2AobQ0kxOqKg+j=CuOWX=RLB_i8hxg at mail dot gmail dot com> <CAMe9rOoWy+hqiCOKqFx0nOFVRT_kBETJ5hYE+cY6pFWUke=tkw at mail dot gmail dot com> <CAMXFM3vG1DNELfGaOOoUvRDGgrGWL4m3M8+5ngPg8RPnCLqUog at mail dot gmail dot com>
On Tue, Feb 16, 2016 at 4:04 AM, Andrew Senkevich
<andrew.n.senkevich@gmail.com> wrote:
> 2016-02-11 20:25 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
>> On Thu, Feb 11, 2016 at 9:21 AM, Andrew Senkevich
>> <andrew.n.senkevich@gmail.com> wrote:
>>> 2016-02-11 19:43 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>>> On Thu, 11 Feb 2016, Andrew Senkevich wrote:
>>>>
>>>>> If we need runtime tests with calls to finite aliases it looks better
>>>>> to adopt build of existing libmvec tests. We can call finite aliases
>>>>> in *-wrappers.c and build shared library from them and link with it
>>>>> test binaries. Is this approach looks OK?
>>>>
>>>> I don't think the finite aliases should be considered part of the API to
>>>> test; it's an implementation detail of the header that they may get
>>>> referenced in certain circumstances.
>>>
>>> I agree.
>>>
>>>> The relevant thing to test is
>>>> whether building a program that directly calls the scalar functions, with
>>>> options such that the calls get vectorized, works (including with variant
>>>> options for e.g. LTO).
>>>
>>> But it looks more like compiler test, not library. Or do you mean some
>>> ABI test for vector functions?
>>> In any case is it necessary for this patch or the following patch is
>>> Ok for thrunk?
>>>
>>> diff --git a/ChangeLog b/ChangeLog
>>> index 11c3156..6ebcefb 100644
>>> --- a/ChangeLog
>>> +++ b/ChangeLog
>>> @@ -1,3 +1,9 @@
>>> +2016-02-10 Andrew Senkevich <andrew.senkevich@intel.com>
>>> + Carlos O'Donell <carlos@redhat.com>
>>> +
>>> + [BZ #19590]
>>> + * sysdeps/x86_64/fpu/svml_finite_alias.S (ALIAS_IMPL): Use PLT.
>>> +
>>> 2016-02-04 Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>
>>>
>>> * sysdeps/powerpc/fpu/libm-test-ulps: Regenerated.
>>> diff --git a/sysdeps/x86_64/fpu/svml_finite_alias.S
>>> b/sysdeps/x86_64/fpu/svml_finite_alias.S
>>> index 0062fe4..8314cf4 100644
>>> --- a/sysdeps/x86_64/fpu/svml_finite_alias.S
>>> +++ b/sysdeps/x86_64/fpu/svml_finite_alias.S
>>> @@ -23,8 +23,7 @@
>>>
>>> #define ALIAS_IMPL(alias, target) \
>>> ENTRY (alias); \
>>> - call target; \
>>> - ret; \
>>> + jmp target@PLT; \
>>> END (alias)
>>>
>>> .text
>>>
>>
>> You need to a test to show your change fixes something.
>
> Here is patch with tests.
>
> diff --git a/ChangeLog b/ChangeLog
> index a2b394e..f88ca52 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,15 @@
> +2016-02-16 Andrew Senkevich <andrew.senkevich@intel.com>
> + H.J. Lu <hongjiu.lu@intel.com>
> +
> + * sysdeps/x86_64/fpu/Makefile: Added new tests.
> + * sysdeps/x86_64/fpu/svml_finite_alias.S (ALIAS_IMPL):: Use PLT.
> + * sysdeps/x86_64/fpu/test-libmvec-alias-mod.c: New.
> + * sysdeps/x86_64/fpu/test-libmvec-alias.c: Likewise.
> + * sysdeps/x86_64/fpu/test-libmvec-alias-avx512-mod.c: Likewise.
> + * sysdeps/x86_64/fpu/test-libmvec-alias-avx512.c: Likewise.
> + * sysdeps/x86_64/fpu/test-libmvec-alias-wrappers.c: Likewise.
> + * sysdeps/x86_64/fpu/test-libmvec-alias-avx512-wrappers.c: Likewise.
> +
> 2016-02-14 Carlos O'Donelll <carlos@redhat.com>
>
> * manual/install.texi: Latest tested is GCC 5.3, texinfo 6.0, gawk
> diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
> index 88742fa..1db3532 100644
> --- a/sysdeps/x86_64/fpu/Makefile
> +++ b/sysdeps/x86_64/fpu/Makefile
> @@ -29,10 +29,22 @@ endif
> ifeq ($(subdir),math)
> ifeq ($(build-mathvec),yes)
> libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \
> - float-vlen4 float-vlen8 float-vlen8-avx2
> + float-vlen4 float-vlen8 float-vlen8-avx2 libmvec-alias
> +modules-names += test-libmvec-alias-mod
> +test-libmvec-alias-mod.so-no-z-defs = yes
> +
> +$(objpfx)test-libmvec-alias: $(objpfx)test-libmvec-alias-mod.so
> +$(objpfx)test-libmvec-alias-mod.so: $(objpfx)../mathvec/libmvec_nonshared.a \
> + $(libmvec)
>
> ifeq (yes,$(config-cflags-avx512))
> -libmvec-tests += double-vlen8 float-vlen16
> +libmvec-tests += double-vlen8 float-vlen16 libmvec-alias-avx512
> +modules-names += test-libmvec-alias-avx512-mod
> +test-libmvec-alias-avx512-mod.so-no-z-defs = yes
> +
> +$(objpfx)test-libmvec-alias-avx512: $(objpfx)test-libmvec-alias-avx512-mod.so
> +$(objpfx)test-libmvec-alias-avx512-mod.so: \
> + $(objpfx)../mathvec/libmvec_nonshared.a $(libmvec)
> endif
>
> double-vlen4-arch-ext-cflags = -mavx
> @@ -43,6 +55,9 @@ float-vlen8-arch-ext-cflags = -mavx
> float-vlen8-arch-ext2-cflags = -mavx2
> float-vlen16-arch-ext-cflags = -mavx512f
>
> +CFLAGS-test-libmvec-alias-mod.c = $(double-vlen4-arch-ext2-cflags)
> +CFLAGS-test-libmvec-alias-avx512-mod.c = $(double-vlen8-arch-ext-cflags)
> +
> CFLAGS-test-double-vlen4-avx2.c = $(libm-test-vec-cflags)
> CFLAGS-test-double-vlen4-avx2-wrappers.c = $(double-vlen4-arch-ext2-cflags)
>
> diff --git a/sysdeps/x86_64/fpu/svml_finite_alias.S
> b/sysdeps/x86_64/fpu/svml_finite_alias.S
> index 0062fe4..8314cf4 100644
> --- a/sysdeps/x86_64/fpu/svml_finite_alias.S
> +++ b/sysdeps/x86_64/fpu/svml_finite_alias.S
> @@ -23,8 +23,7 @@
>
> #define ALIAS_IMPL(alias, target) \
> ENTRY (alias); \
> - call target; \
> - ret; \
> + jmp target@PLT; \
> END (alias)
>
> .text
Please make a small change:
#define ALIAS_IMPL(alias, target) \
ENTRY (alias); \
jmp *target@GOTPCREL(%rip); \
END (alias)
That is using "jmp *target@GOTPCREL(%rip);" instead of "jmp target@PLT;".
It removes one indirect branch in DSO and the new binutils will turn it into
direct branch if `target' is defined locally.
OK with this change.
Thanks.
H.J.