Message ID | CAMXFM3sjjaAn5gudXq8TDL1xaDHhzc7k1SySFV-aX2H=648kRQ@mail.gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 26675 invoked by alias); 30 Sep 2014 18:40:49 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 26653 invoked by uid 89); 30 Sep 2014 18:40:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.5 required=5.0 tests=AWL, BAYES_50, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS, URIBL_BLACK autolearn=no version=3.3.2 X-HELO: mail-qc0-f182.google.com X-Received: by 10.224.120.70 with SMTP id c6mr64680762qar.28.1412102442585; Tue, 30 Sep 2014 11:40:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <Pine.LNX.4.64.1409301620020.15186@digraph.polyomino.org.uk> References: <CAMXFM3tjquzniXP1weqxSVFJyhXqsf2PHuyrrrmqp7K0ZzORqA@mail.gmail.com> <CAMXFM3sGMNX1DEPAMt7qUR4UREF_xUAQjCG1OjBiZH2aoOFiPA@mail.gmail.com> <Pine.LNX.4.64.1409181551370.31607@digraph.polyomino.org.uk> <CAMXFM3tO7MTYCq8-YFZacdbLvR4iAab_n04AuB+rp2Phs4BvQg@mail.gmail.com> <Pine.LNX.4.64.1409242011260.7597@digraph.polyomino.org.uk> <CAMXFM3tqiqUNuSU2KXvAFM-QescX3+6xUO9=z5X0Ac6C9qJ7zg@mail.gmail.com> <CAMe9rOq7bZHb8R=opUzSmAMGWjLpX21mR=Sx96cuBph=TTtDXA@mail.gmail.com> <54246CB5.7020908@redhat.com> <CAMe9rOoLmJ2jHWmERoB0M83WNKovJOgh0--Kquw9O86A1tPU0g@mail.gmail.com> <5424733D.6010305@redhat.com> <CAMe9rOpacze055qyBFzz3M-b-GNtXCqZzMmkScBL9a94zVj28g@mail.gmail.com> <54247FAB.6050002@redhat.com> <CAMXFM3v8narOLMHC5U=fvyTFWV6s4ZACN-UrAC4fAcUs9SOFfA@mail.gmail.com> <54257507.9070508@redhat.com> <CAMXFM3vOLspQtHxgJfD_Emht480w2RMbiwnEH6A_LhoS-JZFag@mail.gmail.com> <Pine.LNX.4.64.1409301620020.15186@digraph.polyomino.org.uk> From: Andrew Senkevich <andrew.n.senkevich@gmail.com> Date: Tue, 30 Sep 2014 22:40:12 +0400 Message-ID: <CAMXFM3sjjaAn5gudXq8TDL1xaDHhzc7k1SySFV-aX2H=648kRQ@mail.gmail.com> Subject: Re: [RFC] How to add vector math functions to Glibc To: "Joseph S. Myers" <joseph@codesourcery.com> Cc: "Carlos O'Donell" <carlos@redhat.com>, libc-alpha <libc-alpha@sourceware.org> Content-Type: text/plain; charset=UTF-8 |
Commit Message
Andrew Senkevich
Sept. 30, 2014, 6:40 p.m. UTC
2014-09-30 20:35 GMT+04:00 Joseph S. Myers <joseph@codesourcery.com>: > On Tue, 30 Sep 2014, Andrew Senkevich wrote: > >> diff --git a/configure.ac b/configure.ac >> index 82d0896..c5c1758 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -903,7 +903,7 @@ LIBC_PROG_BINUTILS >> # Accept binutils 2.20 or newer. >> AC_CHECK_PROG_VER(AS, $AS, --version, >> [GNU assembler.* \([0-9]*\.[0-9.]*\)], >> - [2.1[0-9][0-9]*|2.[2-9][0-9]*|[3-9].*|[1-9][0-9]*], AS=: >> critic_missing="$critic_missing as") >> + [2.1[0-9][0-9]*|2.[2-9][2-9]*|[3-9].*|[1-9][0-9]*], AS=: >> critic_missing="$critic_missing as") >> AC_CHECK_PROG_VER(LD, $LD, --version, >> [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)], >> [2.1[0-9][0-9]*|2.[2-9][0-9]*|[3-9].*|[1-9][0-9]*], LD=: >> critic_missing="$critic_missing ld") > > Any change to required versions needs to include an update to install.texi > (and the generated INSTALL file). It should also be proposed in a > separate thread whose subject describes what is being proposed. I thought it is already agreed in https://sourceware.org/ml/libc-alpha/2014-09/msg00586.html But if separate thread is required I can start it. >> +# We need to install libm.so as linker script >> +# for more comfortable use of vector math library. >> +subdir_install: $(inst_libdir)/libm.so >> + >> +$(inst_libdir)/libm.so: $(common-objpfx)format.lds \ >> + $(common-objpfx)math/libm.so$(libm.so-version) \ >> + $(common-objpfx)mathvec/libmvec.so$(libmvec.so-version) \ >> + $(+force) >> + (echo '/* GNU ld script */';\ >> + cat $<; \ >> + echo 'GROUP ( $(slibdir)/libm.so$(libm.so-version) ' \ >> + 'AS_NEEDED ( $(slibdir)/libmvec.so$(libmvec.so-version) ) )' \ >> + ) > $@.new >> + mv -f $@.new $@ > > Do you have ordering issues here? It seems bad for math/ to install a > direct symlink and then mathvec/ to change it to something else - all > installation rules for libm should be in the math/ directory. It must be in another Makefile of course. > Do you need to link libmvec against libm (and if so, I'd expect associated > Makefile rules, and maybe a Depend file to ensure the directories are > built in the right order)? Libmvec contains calls to scalar version from libm, but not supposed to be used directly. Is it ok not to link libmvec against libm in this case? > Also, I'm not sure the empty libmvec option for unsupported architectures > when we consider the case where functions require GCC or binutils versions > newer than we wish to require, so they are optional on some architecture. > I think having libmvec built or not built on that architecture, depending > on the tools installed, is better than possibly having it built but empty > if the tools are too old. If library is empty but headers installed it will cause compilation fail with according options. Is it OK to add configure option enabled by default on x86_64 and disabled on unsupported architectures? > Did the patch pass the testsuite? If so, you have a problem - you didn't > add ABI test baselines for this library (in this version, a default empty > baseline, and one in sysdeps/unix/sysv/linux/x86_64), so the ABI tests > should have failed, and you need to find out why they didn't run for this > library, and fix that. If it failed for lack of ABI test baselines, add > them. Patch didn't pass the testsuite (even I don't mean it as patch, just as RFC). The following will be added: + GLIBC_2.21 A + _ZGVdN4v_cos F >> +#if defined __x86_64__ && defined __FAST_MATH__ >> +# define __DECL_SIMD_AVX2 >> +# define __DECL_SIMD_SSE4 > I don't see the need for this initial define to empty and subsequent > #undef. Except you should probably have comments explaining exactly what > these macros mean in terms of what function versions they define to be > available. If one function added it affects addition of 2 lines (both OpenMP and Cilk Plus cases), in this case it affects addition of only one line. -- WBR, Andrew
Comments
On Tue, 30 Sep 2014, Andrew Senkevich wrote: > 2014-09-30 20:35 GMT+04:00 Joseph S. Myers <joseph@codesourcery.com>: > > On Tue, 30 Sep 2014, Andrew Senkevich wrote: > > > >> diff --git a/configure.ac b/configure.ac > >> index 82d0896..c5c1758 100644 > >> --- a/configure.ac > >> +++ b/configure.ac > >> @@ -903,7 +903,7 @@ LIBC_PROG_BINUTILS > >> # Accept binutils 2.20 or newer. > >> AC_CHECK_PROG_VER(AS, $AS, --version, > >> [GNU assembler.* \([0-9]*\.[0-9.]*\)], > >> - [2.1[0-9][0-9]*|2.[2-9][0-9]*|[3-9].*|[1-9][0-9]*], AS=: > >> critic_missing="$critic_missing as") > >> + [2.1[0-9][0-9]*|2.[2-9][2-9]*|[3-9].*|[1-9][0-9]*], AS=: > >> critic_missing="$critic_missing as") > >> AC_CHECK_PROG_VER(LD, $LD, --version, > >> [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)], > >> [2.1[0-9][0-9]*|2.[2-9][0-9]*|[3-9].*|[1-9][0-9]*], LD=: > >> critic_missing="$critic_missing ld") > > > > Any change to required versions needs to include an update to install.texi > > (and the generated INSTALL file). It should also be proposed in a > > separate thread whose subject describes what is being proposed. > > I thought it is already agreed in > https://sourceware.org/ml/libc-alpha/2014-09/msg00586.html > But if separate thread is required I can start it. In general, patch submissions should be minimal (subject to bisectability) - if pieces can sensibly be separated out, they should be, and each piece should be given a meaningful subject (which will be the summary line of the git commit message) describing what that piece does. It's entirely plausible there are people concerned about a change to build requirements who aren't concerned about vector functions. > > Do you need to link libmvec against libm (and if so, I'd expect associated > > Makefile rules, and maybe a Depend file to ensure the directories are > > built in the right order)? > > Libmvec contains calls to scalar version from libm, but not supposed > to be used directly. > Is it ok not to link libmvec against libm in this case? No. To have proper versioned undefined references, if a library A has an undefined reference to a function from another library B then A must be linked against B; otherwise you get an undefined reference without symbol version specified. > Is it OK to add configure option enabled by default on x86_64 and > disabled on unsupported architectures? I think that would be appropriate. > diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libmvec.abilist > b/sysdeps/unix/sysv/linux/x86_64/64/libmvec.abilist Unless and until there is a reason for the set of symbols in this library to differ between -mx32 and -m64, I think the ABI baseline should go directly in sysdeps/unix/sysv/linux/x86_64/ rather than the 64/ subdirectory.
2014-10-01 0:03 GMT+04:00 Joseph S. Myers <joseph@codesourcery.com>: > On Tue, 30 Sep 2014, Andrew Senkevich wrote: > >> 2014-09-30 20:35 GMT+04:00 Joseph S. Myers <joseph@codesourcery.com>: >> > On Tue, 30 Sep 2014, Andrew Senkevich wrote: >> > >> >> diff --git a/configure.ac b/configure.ac >> >> index 82d0896..c5c1758 100644 >> >> --- a/configure.ac >> >> +++ b/configure.ac >> >> @@ -903,7 +903,7 @@ LIBC_PROG_BINUTILS >> >> # Accept binutils 2.20 or newer. >> >> AC_CHECK_PROG_VER(AS, $AS, --version, >> >> [GNU assembler.* \([0-9]*\.[0-9.]*\)], >> >> - [2.1[0-9][0-9]*|2.[2-9][0-9]*|[3-9].*|[1-9][0-9]*], AS=: >> >> critic_missing="$critic_missing as") >> >> + [2.1[0-9][0-9]*|2.[2-9][2-9]*|[3-9].*|[1-9][0-9]*], AS=: >> >> critic_missing="$critic_missing as") >> >> AC_CHECK_PROG_VER(LD, $LD, --version, >> >> [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)], >> >> [2.1[0-9][0-9]*|2.[2-9][0-9]*|[3-9].*|[1-9][0-9]*], LD=: >> >> critic_missing="$critic_missing ld") >> > >> > Any change to required versions needs to include an update to install.texi >> > (and the generated INSTALL file). It should also be proposed in a >> > separate thread whose subject describes what is being proposed. >> >> I thought it is already agreed in >> https://sourceware.org/ml/libc-alpha/2014-09/msg00586.html >> But if separate thread is required I can start it. > > In general, patch submissions should be minimal (subject to bisectability) > - if pieces can sensibly be separated out, they should be, and each piece > should be given a meaningful subject (which will be the summary line of > the git commit message) describing what that piece does. It's entirely > plausible there are people concerned about a change to build requirements > who aren't concerned about vector functions. Is it OK to send patch with such update, containing also deletion of configure checks about AVX2 support as well as according preprocessor directive for hiding AVX2 codes? May be something else need to be updated? -- WBR, Andrew
On Wed, 1 Oct 2014, Andrew Senkevich wrote: > > In general, patch submissions should be minimal (subject to bisectability) > > - if pieces can sensibly be separated out, they should be, and each piece > > should be given a meaningful subject (which will be the summary line of > > the git commit message) describing what that piece does. It's entirely > > plausible there are people concerned about a change to build requirements > > who aren't concerned about vector functions. > > Is it OK to send patch with such update, containing also deletion of > configure checks about AVX2 support as well as according preprocessor > directive for hiding AVX2 codes? May be something else need to be > updated? I advise keeping architecture-specific removal of configure checks separate from architecture-independent increases in minimum versions. The AVX2 checks appear to be compiler tests, not binutils tests, so they could only be removed after an increase of minimum GCC version for building glibc to 4.7. Again, discussion of minimum GCC versions (architecture-independent) is best done in a separate thread that is explicitly about that question and that question only, but I'm not sure if there would be a consensus for 4.7 or only for 4.6 as new minimum version. And removal of configure checks that are obsolete with the new minimum version might still best be separate from the patch that actually increases the minimum.
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libmvec.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libmvec.abilist new file mode 100644 index 0000000..1d53a6c --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/64/libmvec.abilist @@ -0,0 +1 @@ +GLIBC_2.21