From patchwork Thu Jun 30 12:35:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Senkevich X-Patchwork-Id: 13507 Received: (qmail 3882 invoked by alias); 30 Jun 2016 12:36:30 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 3865 invoked by uid 89); 30 Jun 2016 12:36:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy=andrew.n.senkevich@gmail.com, andrewnsenkevichgmailcom, 6* X-HELO: mail-vk0-f54.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=8i8eORJ3U4x1nWbtx8dRZ/RNGqaOnNrLzpWQTYtcryQ=; b=gP8nmxn3eZTf1EYwk8OIwAOX06Ulf9U0CVTxaX1840sU56NxkH47tqmN4rNNqVjg9e BHmjtPzS6DDc0avmNjdvub4NbrHGdMbG+o/6pTXZN9yelWuKsW9gpS1KlKMEQ2nG3gKi VPX/B28hlpZKxwpNc/cCwu+2jizPOSsUu4HyUIWd/YOBdK5Zb9BGDP2u/0kKUi24tlho IzDPPTReJPSsE9D/n1tsBXShnnEHwJshsvieo2FY6v+ydc3e/miZRewTjKShXLVprUBu AOrYtaWcwRJsA2TrKl4hhjqKwqYvaVNUqirvXWxOUJzEh8gIUEueg3QUR6v2rgCFLbhw kNLQ== X-Gm-Message-State: ALyK8tJMpCJOHhNBG9tdK7/KibImnBipE/X6r7+MRZTLddsOrb0RD8Lm0Oi0h/Gh1vlaS4TbnIGfMV200aPH5A== X-Received: by 10.176.3.1 with SMTP id 1mr5638346uat.139.1467290176671; Thu, 30 Jun 2016 05:36:16 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: From: Andrew Senkevich Date: Thu, 30 Jun 2016 15:35:47 +0300 Message-ID: Subject: Re: [PATCH x86-64][BZ #20024] Fixed vector sincos/sincosf ABI To: Joseph Myers Cc: libc-alpha 2016-06-30 14:43 GMT+03:00 Andrew Senkevich : > 2016-06-06 17:08 GMT+03:00 Joseph Myers : >> On Mon, 6 Jun 2016, Andrew Senkevich wrote: >> >>> 2016-06-03 1:50 GMT+03:00 Joseph Myers : >>> > On Tue, 31 May 2016, Andrew Senkevich wrote: >>> > >>> >> Hi, >>> >> >>> >> this patch fixes wrong vector sincos/sincosf ABI to have it compatible with >>> >> current vector function declaration. According to current vector function >>> >> declaration vectorized sincos should have vector of pointers for second and >>> >> third parameters, so it is fixed with implementation as wrapper to version >>> >> having second and third parameters as pointers. >>> >> Is it Ok for trunk, 2.22 and 2.23 releases branches? >>> > >>> > Do you intend a followup for trunk only that exports the new functions >>> > with the intended ABI and makes the old ones into compat symbols? >>> >>> Is it suitable way to have both simd declarations for sincos in headers? >> >> (a) Would that work usefully, and cause both functions to be used >> depending on the code to be vectorized? >> >> (b) How useful are the existing functions, i.e. would real code be likely >> to use both functions? > > It is hard to say about real code, but GCC 6.1 can vectorize both > cases depending on user code. > > So we can have both versions and test them both with the following > patch (after main patch from this thread): > > diff --git a/sysdeps/unix/sysv/linux/x86_64/libmvec.abilist > b/sysdeps/unix/sysv/linux/x86_64/libmvec.abilist > index 80d028a..e3e450c 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/libmvec.abilist > +++ b/sysdeps/unix/sysv/linux/x86_64/libmvec.abilist > @@ -47,3 +47,12 @@ GLIBC_2.22 _ZGVeN8v_log F > GLIBC_2.22 _ZGVeN8v_sin F > GLIBC_2.22 _ZGVeN8vv_pow F > GLIBC_2.22 _ZGVeN8vvv_sincos F > +GLIBC_2.24 GLIBC_2.24 A > +GLIBC_2.24 _ZGVbN2vl8l8_sincos F > +GLIBC_2.24 _ZGVbN4vl4l4_sincosf F > +GLIBC_2.24 _ZGVcN4vl8l8_sincos F > +GLIBC_2.24 _ZGVcN8vl4l4_sincosf F > +GLIBC_2.24 _ZGVdN4vl8l8_sincos F > +GLIBC_2.24 _ZGVdN8vl4l4_sincosf F > +GLIBC_2.24 _ZGVeN16vl4l4_sincosf F > +GLIBC_2.24 _ZGVeN8vl8l8_sincos F > diff --git a/sysdeps/x86/fpu/bits/math-vector.h > b/sysdeps/x86/fpu/bits/math-vector.h > index ca43cf4..74a6bf8 100644 > --- a/sysdeps/x86/fpu/bits/math-vector.h > +++ b/sysdeps/x86/fpu/bits/math-vector.h > @@ -43,9 +43,9 @@ > # undef __DECL_SIMD_sinf > # define __DECL_SIMD_sinf __DECL_SIMD_x86_64 > # undef __DECL_SIMD_sincos > -# define __DECL_SIMD_sincos __DECL_SIMD_x86_64 > +# define __DECL_SIMD_sincos __DECL_SIMD_x86_64 _Pragma ("omp declare > simd notinbranch linear (__sinx, __cosx: 1)") > # undef __DECL_SIMD_sincosf > -# define __DECL_SIMD_sincosf __DECL_SIMD_x86_64 > +# define __DECL_SIMD_sincosf __DECL_SIMD_x86_64 _Pragma ("omp > declare simd notinbranch linear (__sinx, __cosx: 1)") > # undef __DECL_SIMD_log > # define __DECL_SIMD_log __DECL_SIMD_x86_64 > # undef __DECL_SIMD_logf > diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile > index 25aef40..ee281c2 100644 > --- a/sysdeps/x86_64/fpu/Makefile > +++ b/sysdeps/x86_64/fpu/Makefile > @@ -170,8 +170,8 @@ float-vlen8-arch-ext-cflags = -mavx > float-vlen8-arch-ext2-cflags = -mavx2 > float-vlen16-arch-ext-cflags = -mavx512f > > -libmvec-sincos-cflags = $(libm-test-fast-math-cflags) -fopenmp > -Wno-unknown-pragmas > -libmvec-alias-cflags = $(libmvec-sincos-cflags) -fno-inline > -ffloat-store -ffinite-math-only > +libmvec-sincos-cflags = $(libm-test-fast-math-cflags) -fopenmp > -fno-inline -Wno-unknown-pragmas > +libmvec-alias-cflags = $(libmvec-sincos-cflags) -ffloat-store > -ffinite-math-only > > CFLAGS-test-double-libmvec-alias-mod.c = $(libmvec-alias-cflags) > CFLAGS-test-double-libmvec-alias-avx-mod.c = > $(double-vlen4-arch-ext-cflags) $(libmvec-alias-cflags) -DREQUIRE_AVX > diff --git a/sysdeps/x86_64/fpu/Versions b/sysdeps/x86_64/fpu/Versions > index 0813204..02df4b5 100644 > --- a/sysdeps/x86_64/fpu/Versions > +++ b/sysdeps/x86_64/fpu/Versions > @@ -13,4 +13,8 @@ libmvec { > _ZGVbN4vv_powf; _ZGVcN8vv_powf; _ZGVdN8vv_powf; _ZGVeN16vv_powf; > _ZGVbN4vvv_sincosf; _ZGVcN8vvv_sincosf; _ZGVdN8vvv_sincosf; > _ZGVeN16vvv_sincosf; > } > + GLIBC_2.24 { > + _ZGVbN2vl8l8_sincos; _ZGVcN4vl8l8_sincos; _ZGVdN4vl8l8_sincos; > _ZGVeN8vl8l8_sincos; > + _ZGVbN4vl4l4_sincosf; _ZGVcN8vl4l4_sincosf; _ZGVdN8vl4l4_sincosf; > _ZGVeN16vl4l4_sincosf; > + } > } > diff --git a/sysdeps/x86_64/fpu/test-double-libmvec-sincos.c > b/sysdeps/x86_64/fpu/test-double-libmvec-sincos.c > index 80348a2..8fe106d 100644 > --- a/sysdeps/x86_64/fpu/test-double-libmvec-sincos.c > +++ b/sysdeps/x86_64/fpu/test-double-libmvec-sincos.c > @@ -21,6 +21,7 @@ > > #define N 1000 > double x[N], s[N], c[N]; > +double x1[N], s1[N], c1[N]; > double* s_ptrs[N]; > double* c_ptrs[N]; > int arch_check = 1; > @@ -28,15 +29,13 @@ int arch_check = 1; > static void > init_arg (void) > { > - int i; > - > CHECK_ARCH_EXT; > > arch_check = 0; > > - for(i = 0; i < N; i++) > + for(int i = 0; i < N; i++) > { > - x[i] = i / 3; > + x[i] = x1[i] = i / 3; > s_ptrs[i] = &s[i]; > c_ptrs[i] = &c[i]; > } > @@ -45,16 +44,19 @@ init_arg (void) > static int > test_sincos_abi (void) > { > - int i; > - > - init_arg (); > +#pragma omp simd > + for(int i = 0; i < N; i++) > + sincos (x[i], s_ptrs[i], c_ptrs[i]); > > - if (arch_check) > - return 77; > + return 0; > +} > > +static int > +test_sincos_linear_abi (void) > +{ > #pragma omp simd > - for(i = 0; i < N; i++) > - sincos (x[i], s_ptrs[i], c_ptrs[i]); > + for(int i = 0; i < N; i++) > + sincos (x1[i], &s1[i], &c1[i]); > > return 0; > } > @@ -62,7 +64,16 @@ test_sincos_abi (void) > static int > do_test (void) > { > - return test_sincos_abi (); > + init_arg (); > + > + if (arch_check) > + return 77; > + > + test_sincos_abi (); > + > + test_sincos_linear_abi (); > + > + return 0; > } > > #define TEST_FUNCTION do_test () > > (The same change for sysdeps/x86_64/fpu/test-float-libmvec-sincosf.c) > > Is this change Ok generally? More correct change for sysdeps/x86/fpu/bits/math-vector.h is: # ifdef __DECL_SIMD_x86_64 @@ -43,9 +45,9 @@ # undef __DECL_SIMD_sinf # define __DECL_SIMD_sinf __DECL_SIMD_x86_64 # undef __DECL_SIMD_sincos -# define __DECL_SIMD_sincos __DECL_SIMD_x86_64 +# define __DECL_SIMD_sincos __DECL_SIMD_x86_64_sincos # undef __DECL_SIMD_sincosf -# define __DECL_SIMD_sincosf __DECL_SIMD_x86_64 +# define __DECL_SIMD_sincosf __DECL_SIMD_x86_64_sincos # undef __DECL_SIMD_log # define __DECL_SIMD_log __DECL_SIMD_x86_64 # undef __DECL_SIMD_logf --- WBR, Andrew diff --git a/sysdeps/x86/fpu/bits/math-vector.h b/sysdeps/x86/fpu/bits/math-vector.h index ca43cf4..c20715b --- a/sysdeps/x86/fpu/bits/math-vector.h +++ b/sysdeps/x86/fpu/bits/math-vector.h @@ -28,9 +28,11 @@ # if defined _OPENMP && _OPENMP >= 201307 /* OpenMP case. */ # define __DECL_SIMD_x86_64 _Pragma ("omp declare simd notinbranch") +# define __DECL_SIMD_x86_64_sincos __DECL_SIMD_x86_64 _Pragma ("omp declare simd notinbranch linear (__sinx, __cosx: 1)") # elif __GNUC_PREREQ (6,0) /* W/o OpenMP use GCC 6.* __attribute__ ((__simd__)). */ # define __DECL_SIMD_x86_64 __attribute__ ((__simd__ ("notinbranch"))) +# define __DECL_SIMD_x86_64_sincos __DECL_SIMD_x86_64 # endif