diff mbox

BZ #19590: Fixed build of shared objects that use libmvec.so functions

Message ID CAMXFM3vG1DNELfGaOOoUvRDGgrGWL4m3M8+5ngPg8RPnCLqUog@mail.gmail.com
State New, archived
Headers show

Commit Message

Andrew Senkevich Feb. 16, 2016, 12:04 p.m. UTC
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.


Is it Ok for trunk?


--
WBR,
Andrew

Comments

H.J. Lu Feb. 16, 2016, 1:09 p.m. UTC | #1
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.
Joseph Myers Feb. 16, 2016, 1:49 p.m. UTC | #2
On Tue, 16 Feb 2016, Andrew Senkevich wrote:

> Here is patch with tests.

This is the wrong approach for tests.  Tests for this should not be 
testing implementation details about aliases, and so should not be 
creating any wrappers at all.  They should be testing vectorizable calls 
to the scalar functions, compiled several times with different options 
into both executables and shared libraries.
H.J. Lu Feb. 16, 2016, 2:23 p.m. UTC | #3
On Tue, Feb 16, 2016 at 5:49 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>
>> Here is patch with tests.
>
> This is the wrong approach for tests.  Tests for this should not be
> testing implementation details about aliases, and so should not be
> creating any wrappers at all.  They should be testing vectorizable calls
> to the scalar functions, compiled several times with different options
> into both executables and shared libraries.
>

Andrew, please update BZ 19590 with a real GCC testcase to show
the problem.

You can need [BZ #19590] in ChangeLog.
Andrew Senkevich Feb. 17, 2016, 2:14 p.m. UTC | #4
2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>
>> Here is patch with tests.
>
> This is the wrong approach for tests.  Tests for this should not be
> testing implementation details about aliases, and so should not be
> creating any wrappers at all.  They should be testing vectorizable calls
> to the scalar functions, compiled several times with different options
> into both executables and shared libraries.

Please look at attached version.


--
WBR,
Andrew
H.J. Lu Feb. 17, 2016, 2:29 p.m. UTC | #5
On Wed, Feb 17, 2016 at 6:14 AM, Andrew Senkevich
<andrew.n.senkevich@gmail.com> wrote:
> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>>
>>> Here is patch with tests.
>>
>> This is the wrong approach for tests.  Tests for this should not be
>> testing implementation details about aliases, and so should not be
>> creating any wrappers at all.  They should be testing vectorizable calls
>> to the scalar functions, compiled several times with different options
>> into both executables and shared libraries.
>
> Please look at attached version.

There may be 2 issues:

1. GCC 4.X may not support pragma simd.
2. GCC X may call those aliases.

I think test-libmvec-alias-mod.c should be in assembly.
Andrew Senkevich Feb. 17, 2016, 2:37 p.m. UTC | #6
2016-02-17 17:29 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
> On Wed, Feb 17, 2016 at 6:14 AM, Andrew Senkevich
> <andrew.n.senkevich@gmail.com> wrote:
>> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>>>
>>>> Here is patch with tests.
>>>
>>> This is the wrong approach for tests.  Tests for this should not be
>>> testing implementation details about aliases, and so should not be
>>> creating any wrappers at all.  They should be testing vectorizable calls
>>> to the scalar functions, compiled several times with different options
>>> into both executables and shared libraries.
>>
>> Please look at attached version.
>
> There may be 2 issues:
>
> 1. GCC 4.X may not support pragma simd.
> 2. GCC X may call those aliases.
>
> I think test-libmvec-alias-mod.c should be in assembly.

For 1. we use -Wno-unknown-pragmas in make rules.

> 2. GCC X may call those aliases.

Do you mean GCC X may don't call those aliases?
Yes, but then it will use not alias version and test will be also work.



--
WBR,
Andrew
H.J. Lu Feb. 17, 2016, 3:04 p.m. UTC | #7
On Wed, Feb 17, 2016 at 6:37 AM, Andrew Senkevich
<andrew.n.senkevich@gmail.com> wrote:
> 2016-02-17 17:29 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
>> On Wed, Feb 17, 2016 at 6:14 AM, Andrew Senkevich
>> <andrew.n.senkevich@gmail.com> wrote:
>>> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>>> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>>>>
>>>>> Here is patch with tests.
>>>>
>>>> This is the wrong approach for tests.  Tests for this should not be
>>>> testing implementation details about aliases, and so should not be
>>>> creating any wrappers at all.  They should be testing vectorizable calls
>>>> to the scalar functions, compiled several times with different options
>>>> into both executables and shared libraries.
>>>
>>> Please look at attached version.
>>
>> There may be 2 issues:
>>
>> 1. GCC 4.X may not support pragma simd.
>> 2. GCC X may call those aliases.
>>
>> I think test-libmvec-alias-mod.c should be in assembly.
>
> For 1. we use -Wno-unknown-pragmas in make rules.

Then those tests will be skipped.

>> 2. GCC X may call those aliases.
>
> Do you mean GCC X may don't call those aliases?
> Yes, but then it will use not alias version and test will be also work.

Then those tests aren't effective for GCC X.
Andrew Senkevich Feb. 17, 2016, 3:22 p.m. UTC | #8
2016-02-17 18:04 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
> On Wed, Feb 17, 2016 at 6:37 AM, Andrew Senkevich
> <andrew.n.senkevich@gmail.com> wrote:
>> 2016-02-17 17:29 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
>>> On Wed, Feb 17, 2016 at 6:14 AM, Andrew Senkevich
>>> <andrew.n.senkevich@gmail.com> wrote:
>>>> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>>>> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>>>>>
>>>>>> Here is patch with tests.
>>>>>
>>>>> This is the wrong approach for tests.  Tests for this should not be
>>>>> testing implementation details about aliases, and so should not be
>>>>> creating any wrappers at all.  They should be testing vectorizable calls
>>>>> to the scalar functions, compiled several times with different options
>>>>> into both executables and shared libraries.
>>>>
>>>> Please look at attached version.
>>>
>>> There may be 2 issues:
>>>
>>> 1. GCC 4.X may not support pragma simd.
>>> 2. GCC X may call those aliases.
>>>
>>> I think test-libmvec-alias-mod.c should be in assembly.
>>
>> For 1. we use -Wno-unknown-pragmas in make rules.
>
> Then those tests will be skipped.
>
>>> 2. GCC X may call those aliases.
>>
>> Do you mean GCC X may don't call those aliases?
>> Yes, but then it will use not alias version and test will be also work.
>
> Then those tests aren't effective for GCC X.

But it looks enough suitable, if compiler can't generate alias name no
needed to test that alias. Non alias name will be used.
We can add other vector functions to this test also to test all of
them, not only versions with aliases.


--
WBR,
Andrew
H.J. Lu Feb. 17, 2016, 3:51 p.m. UTC | #9
On Wed, Feb 17, 2016 at 7:22 AM, Andrew Senkevich
<andrew.n.senkevich@gmail.com> wrote:
> 2016-02-17 18:04 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
>> On Wed, Feb 17, 2016 at 6:37 AM, Andrew Senkevich
>> <andrew.n.senkevich@gmail.com> wrote:
>>> 2016-02-17 17:29 GMT+03:00 H.J. Lu <hjl.tools@gmail.com>:
>>>> On Wed, Feb 17, 2016 at 6:14 AM, Andrew Senkevich
>>>> <andrew.n.senkevich@gmail.com> wrote:
>>>>> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>>>>> On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>>>>>>
>>>>>>> Here is patch with tests.
>>>>>>
>>>>>> This is the wrong approach for tests.  Tests for this should not be
>>>>>> testing implementation details about aliases, and so should not be
>>>>>> creating any wrappers at all.  They should be testing vectorizable calls
>>>>>> to the scalar functions, compiled several times with different options
>>>>>> into both executables and shared libraries.
>>>>>
>>>>> Please look at attached version.
>>>>
>>>> There may be 2 issues:
>>>>
>>>> 1. GCC 4.X may not support pragma simd.
>>>> 2. GCC X may call those aliases.
>>>>
>>>> I think test-libmvec-alias-mod.c should be in assembly.
>>>
>>> For 1. we use -Wno-unknown-pragmas in make rules.
>>
>> Then those tests will be skipped.
>>
>>>> 2. GCC X may call those aliases.
>>>
>>> Do you mean GCC X may don't call those aliases?
>>> Yes, but then it will use not alias version and test will be also work.
>>
>> Then those tests aren't effective for GCC X.
>
> But it looks enough suitable, if compiler can't generate alias name no
> needed to test that alias. Non alias name will be used.
> We can add other vector functions to this test also to test all of
> them, not only versions with aliases.

GCC used for glibc build != GCC used with glibc and differet
GCC can be used to build glibc.  If we can't reliably test glibc
with all GCC supported for glibc build, we should skip the test.
Joseph Myers Feb. 17, 2016, 5:11 p.m. UTC | #10
On Wed, 17 Feb 2016, H.J. Lu wrote:

> I think test-libmvec-alias-mod.c should be in assembly.

That would defeat the point of these tests.
Joseph Myers Feb. 17, 2016, 5:17 p.m. UTC | #11
On Wed, 17 Feb 2016, Andrew Senkevich wrote:

> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
> > On Tue, 16 Feb 2016, Andrew Senkevich wrote:
> >
> >> Here is patch with tests.
> >
> > This is the wrong approach for tests.  Tests for this should not be
> > testing implementation details about aliases, and so should not be
> > creating any wrappers at all.  They should be testing vectorizable calls
> > to the scalar functions, compiled several times with different options
> > into both executables and shared libraries.
> 
> Please look at attached version.

I'm concerned about the change to use -fopenmp in libm-test-vec-cflags.  
That's deliberately using defines to cause declarations to be visible 
rather than other options such as -ffast-math that might have undesired 
side effects.

(In this case, would it introduce any libgomp dependencies?  If libgomp 
needs copying to the glibc build directory for testing like libgcc_s and 
libstdc++ do, in cases where it's not available in a directory searched by 
default, that's a significant change to testing requirements that would 
need to be carefully thought out.  If -fopenmp is only used when compiling 
and not when linking and there are no libgomp dependencies involved, it 
may be safer, but it still needs careful consideration and testing and so 
is unsuitable so close to a release.  And it would seem best in any case 
to use -fopenmp if needed only for the new tests, not for the ones using 
wrappers.)

The tests also seem to fail to follow the GNU Coding Standards in various 
places, such as regarding spaces before '(' and putting function names at 
the start of a new line in function definitions.
Alexander Monakov Feb. 17, 2016, 5:30 p.m. UTC | #12
On Wed, 17 Feb 2016, Joseph Myers wrote:
> (In this case, would it introduce any libgomp dependencies?  ...)

Using -fopenmp-simd instead of plain -fopenmp may be better: it ignores
non-simd OpenMP pragmas and does not attempt to link libgomp.

(otherwise if -fopenmp is present at link time, it implicitely adds -lgomp to
the linker command line)

Alexander
Andrew Senkevich Feb. 17, 2016, 6:20 p.m. UTC | #13
2016-02-17 20:17 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
> On Wed, 17 Feb 2016, Andrew Senkevich wrote:
>
>> 2016-02-16 16:49 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>> > On Tue, 16 Feb 2016, Andrew Senkevich wrote:
>> >
>> >> Here is patch with tests.
>> >
>> > This is the wrong approach for tests.  Tests for this should not be
>> > testing implementation details about aliases, and so should not be
>> > creating any wrappers at all.  They should be testing vectorizable calls
>> > to the scalar functions, compiled several times with different options
>> > into both executables and shared libraries.
>>
>> Please look at attached version.
>
> I'm concerned about the change to use -fopenmp in libm-test-vec-cflags.
> That's deliberately using defines to cause declarations to be visible
> rather than other options such as -ffast-math that might have undesired
> side effects.
>
> (In this case, would it introduce any libgomp dependencies?  If libgomp
> needs copying to the glibc build directory for testing like libgcc_s and
> libstdc++ do, in cases where it's not available in a directory searched by
> default, that's a significant change to testing requirements that would
> need to be carefully thought out.  If -fopenmp is only used when compiling
> and not when linking and there are no libgomp dependencies involved, it
> may be safer, but it still needs careful consideration and testing and so
> is unsuitable so close to a release.  And it would seem best in any case
> to use -fopenmp if needed only for the new tests, not for the ones using
> wrappers.)

I didn't see libgomp dependency.  I stay -fopenmp only for new tests.

Is attached version finally Ok for trunk?


--
WBR,
Andrew
Andrew Senkevich Feb. 17, 2016, 6:24 p.m. UTC | #14
2016-02-17 20:30 GMT+03:00 Alexander Monakov <amonakov@ispras.ru>:
> On Wed, 17 Feb 2016, Joseph Myers wrote:
>> (In this case, would it introduce any libgomp dependencies?  ...)
>
> Using -fopenmp-simd instead of plain -fopenmp may be better: it ignores
> non-simd OpenMP pragmas and does not attempt to link libgomp.
>
> (otherwise if -fopenmp is present at link time, it implicitely adds -lgomp to
> the linker command line)

With -fopenmp-simd GCC 4.9 don't vectorize needed calls.


--
WBR,
Andrew
Joseph Myers Feb. 17, 2016, 6:44 p.m. UTC | #15
On Wed, 17 Feb 2016, Andrew Senkevich wrote:

> Is attached version finally Ok for trunk?

Please resubmit, with the coding style issues fixed and tests moved to 
test-skeleton.c unless there's a reason they can't use it, after the 
freeze is over.  This is much too risky to consider during the freeze, and 
would need to wait a while on master before any backports could be 
considered (e.g. the new tests could break the testsuite build in some 
configurations for unforeseen reasons).
Andrew Senkevich Feb. 17, 2016, 7:30 p.m. UTC | #16
2016-02-17 21:44 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
> On Wed, 17 Feb 2016, Andrew Senkevich wrote:
>
>> Is attached version finally Ok for trunk?
>
> Please resubmit, with the coding style issues fixed and tests moved to
> test-skeleton.c unless there's a reason they can't use it, after the
> freeze is over.  This is much too risky to consider during the freeze, and
> would need to wait a while on master before any backports could be
> considered (e.g. the new tests could break the testsuite build in some
> configurations for unforeseen reasons).

May be separate fix and tests to commit fix earlier?


--
WBR,
Andrew
H.J. Lu Feb. 17, 2016, 8:06 p.m. UTC | #17
On Wed, Feb 17, 2016 at 11:30 AM, Andrew Senkevich
<andrew.n.senkevich@gmail.com> wrote:
> 2016-02-17 21:44 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>> On Wed, 17 Feb 2016, Andrew Senkevich wrote:
>>
>>> Is attached version finally Ok for trunk?
>>
>> Please resubmit, with the coding style issues fixed and tests moved to
>> test-skeleton.c unless there's a reason they can't use it, after the
>> freeze is over.  This is much too risky to consider during the freeze, and
>> would need to wait a while on master before any backports could be
>> considered (e.g. the new tests could break the testsuite build in some
>> configurations for unforeseen reasons).
>
> May be separate fix and tests to commit fix earlier?
>

This is a good idea.  Please submit a fix-only patch.

Thanks.
diff mbox

Patch

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
diff --git a/sysdeps/x86_64/fpu/test-libmvec-alias-avx512-mod.c
b/sysdeps/x86_64/fpu/test-libmvec-alias-avx512-mod.c
new file mode 100644
index 0000000..64218f8
--- /dev/null
+++ b/sysdeps/x86_64/fpu/test-libmvec-alias-avx512-mod.c
@@ -0,0 +1,46 @@ 
+/* Part of test to build shared library to ensure link against
+   *_finite aliases from libmvec.
+   Copyright (C) 2016 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/>.  */
+
+#include <immintrin.h>
+
+#include "test-double-vlen8.h"
+#define VEC_TYPE __m512d
+
+VECTOR_WRAPPER (WRAPPER_NAME (log), _ZGVeN8v___log_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (exp), _ZGVeN8v___exp_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (pow), _ZGVeN8vv___pow_finite)
+
+#undef FUNC
+#undef FLOAT
+#undef BUILD_COMPLEX
+#undef TEST_MSG
+#undef CHOOSE
+#undef FUNC_TEST
+#undef VEC_TYPE
+#undef VECTOR_WRAPPER
+#undef VECTOR_WRAPPER_ff
+#undef VEC_SUFF
+#undef VEC_LEN
+
+#include "test-float-vlen16.h"
+#define VEC_TYPE __m512
+
+VECTOR_WRAPPER (WRAPPER_NAME (logf), _ZGVeN16v___logf_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (expf), _ZGVeN16v___expf_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (powf), _ZGVeN16vv___powf_finite)
diff --git a/sysdeps/x86_64/fpu/test-libmvec-alias-avx512-wrappers.c
b/sysdeps/x86_64/fpu/test-libmvec-alias-avx512-wrappers.c
new file mode 100644
index 0000000..9d841c9
--- /dev/null
+++ b/sysdeps/x86_64/fpu/test-libmvec-alias-avx512-wrappers.c
@@ -0,0 +1 @@ 
+/* Dummy file.  */
diff --git a/sysdeps/x86_64/fpu/test-libmvec-alias-avx512.c
b/sysdeps/x86_64/fpu/test-libmvec-alias-avx512.c
new file mode 100644
index 0000000..8d11234
--- /dev/null
+++ b/sysdeps/x86_64/fpu/test-libmvec-alias-avx512.c
@@ -0,0 +1,43 @@ 
+/* Part of test to ensure link against *_finite aliases from libmvec.
+   Copyright (C) 2016 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/>.  */
+
+#include <init-arch.h>
+
+extern double log_vlen8 (double);
+extern double exp_vlen8 (double);
+extern double pow_vlen8 (double, double);
+
+extern float logf_vlen16 (float);
+extern float expf_vlen16 (float);
+extern float powf_vlen16 (float, float);
+
+int main(void)
+{
+  if (!HAS_ARCH_FEATURE (AVX512F_Usable)) return 0;
+
+  if (log_vlen8(1.0) != 0.0
+      || logf_vlen16(1.0) != 0.0) abort();
+
+  if (exp_vlen8(0.0) != 1.0
+      || expf_vlen16(0.0) != 1.0) abort();
+
+  if (pow_vlen8(1.0, 1.0) != 1.0
+      || powf_vlen16(1.0, 1.0) != 1.0) abort();
+
+  return 0;
+}
diff --git a/sysdeps/x86_64/fpu/test-libmvec-alias-mod.c
b/sysdeps/x86_64/fpu/test-libmvec-alias-mod.c
new file mode 100644
index 0000000..37c2d75
--- /dev/null
+++ b/sysdeps/x86_64/fpu/test-libmvec-alias-mod.c
@@ -0,0 +1,86 @@ 
+/* Part of test to build shared library to ensure link against
+   *_finite aliases from libmvec.
+   Copyright (C) 2016 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/>.  */
+
+#include <immintrin.h>
+
+#include "test-double-vlen2.h"
+#define VEC_TYPE __m128d
+
+VECTOR_WRAPPER (WRAPPER_NAME (log), _ZGVbN2v___log_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (exp), _ZGVbN2v___exp_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (pow), _ZGVbN2vv___pow_finite)
+
+#undef VEC_TYPE
+#undef VECTOR_WRAPPER
+#undef VECTOR_WRAPPER_ff
+#undef VEC_SUFF
+#undef VEC_LEN
+
+#include "test-double-vlen4.h"
+#define VEC_TYPE __m256d
+
+VECTOR_WRAPPER (WRAPPER_NAME (log), _ZGVcN4v___log_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (exp), _ZGVcN4v___exp_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (pow), _ZGVcN4vv___pow_finite)
+
+#undef VEC_SUFF
+#define VEC_SUFF _vlen4_avx2
+
+VECTOR_WRAPPER (WRAPPER_NAME (log), _ZGVdN4v___log_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (exp), _ZGVdN4v___exp_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (pow), _ZGVdN4vv___pow_finite)
+
+#undef FUNC
+#undef FLOAT
+#undef BUILD_COMPLEX
+#undef TEST_MSG
+#undef CHOOSE
+#undef FUNC_TEST
+#undef VEC_TYPE
+#undef VECTOR_WRAPPER
+#undef VECTOR_WRAPPER_ff
+#undef VEC_SUFF
+#undef VEC_LEN
+
+#include "test-float-vlen4.h"
+#define VEC_TYPE __m128
+
+VECTOR_WRAPPER (WRAPPER_NAME (logf), _ZGVbN4v___logf_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (expf), _ZGVbN4v___expf_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (powf), _ZGVbN4vv___powf_finite)
+
+#undef VEC_TYPE
+#undef VECTOR_WRAPPER
+#undef VECTOR_WRAPPER_ff
+#undef VEC_SUFF
+#undef VEC_LEN
+
+#include "test-float-vlen8.h"
+#define VEC_TYPE __m256
+
+VECTOR_WRAPPER (WRAPPER_NAME (logf), _ZGVcN8v___logf_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (expf), _ZGVcN8v___expf_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (powf), _ZGVcN8vv___powf_finite)
+
+#undef VEC_SUFF
+#define VEC_SUFF _vlen8_avx2
+
+VECTOR_WRAPPER (WRAPPER_NAME (logf), _ZGVdN8v___logf_finite)
+VECTOR_WRAPPER (WRAPPER_NAME (expf), _ZGVdN8v___expf_finite)
+VECTOR_WRAPPER_ff (WRAPPER_NAME (powf), _ZGVdN8vv___powf_finite)
diff --git a/sysdeps/x86_64/fpu/test-libmvec-alias-wrappers.c
b/sysdeps/x86_64/fpu/test-libmvec-alias-wrappers.c
new file mode 100644
index 0000000..9d841c9
--- /dev/null
+++ b/sysdeps/x86_64/fpu/test-libmvec-alias-wrappers.c
@@ -0,0 +1 @@ 
+/* Dummy file.  */
diff --git a/sysdeps/x86_64/fpu/test-libmvec-alias.c
b/sysdeps/x86_64/fpu/test-libmvec-alias.c
new file mode 100644
index 0000000..429b19e
--- /dev/null
+++ b/sysdeps/x86_64/fpu/test-libmvec-alias.c
@@ -0,0 +1,81 @@ 
+/* Part of test to ensure link against *_finite aliases from libmvec.
+   Copyright (C) 2016 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/>.  */
+
+#include <init-arch.h>
+
+extern double log_vlen2 (double);
+extern double exp_vlen2 (double);
+extern double pow_vlen2 (double, double);
+
+extern double log_vlen4 (double);
+extern double exp_vlen4 (double);
+extern double pow_vlen4 (double, double);
+
+extern double log_vlen4_avx2 (double);
+extern double exp_vlen4_avx2 (double);
+extern double pow_vlen4_avx2 (double, double);
+
+extern float logf_vlen4 (float);
+extern float expf_vlen4 (float);
+extern float powf_vlen4 (float, float);
+
+extern float logf_vlen8 (float);
+extern float expf_vlen8 (float);
+extern float powf_vlen8 (float, float);
+
+extern float logf_vlen8_avx2 (float);
+extern float expf_vlen8_avx2 (float);
+extern float powf_vlen8_avx2 (float, float);
+
+int main(void)
+{
+  if (log_vlen2(1.0) != 0.0
+      || logf_vlen4(1.0) != 0.0) abort();
+
+  if (exp_vlen2(0.0) != 1.0
+      || expf_vlen4(0.0) != 1.0) abort();
+
+  if (pow_vlen2(1.0, 1.0) != 1.0
+      || powf_vlen4(1.0, 1.0) != 1.0) abort();
+
+  if (HAS_ARCH_FEATURE (AVX_Usable))
+    {
+      if (log_vlen4(1.0) != 0.0
+          || logf_vlen8(1.0) != 0.0) abort();
+
+      if (exp_vlen4(0.0) != 1.0
+          || expf_vlen8(0.0) != 1.0) abort();
+
+      if (pow_vlen4(1.0, 1.0) != 1.0
+          || powf_vlen8(1.0, 1.0) != 1.0) abort();
+    }
+
+  if (HAS_ARCH_FEATURE (AVX2_Usable))
+    {
+      if (log_vlen4_avx2(1.0) != 0.0
+          || logf_vlen8_avx2(1.0) != 0.0) abort();
+
+      if (exp_vlen4_avx2(0.0) != 1.0
+          || expf_vlen8_avx2(0.0) != 1.0) abort();
+
+      if (pow_vlen4_avx2(1.0, 1.0) != 1.0
+          || powf_vlen8_avx2(1.0, 1.0) != 1.0) abort();
+    }
+
+  return 0;
+}