Re: [PATCH] BZ #19590: Fixed build of shared objects that use libmvec.so functions

["H.J. Lu" <hjl dot tools at 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.