Message ID | 20140812183008.GO9284@spoyarek.pnq.redhat.com |
---|---|
State | Committed |
Headers |
Received: (qmail 19752 invoked by alias); 12 Aug 2014 18:30:15 -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 19736 invoked by uid 89); 12 Aug 2014 18:30:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Date: Wed, 13 Aug 2014 00:00:08 +0530 From: Siddhesh Poyarekar <siddhesh@redhat.com> To: libc-alpha@sourceware.org Cc: Jakub Jelinek <jakub@redhat.com> Subject: [PATCH] Disable x87 inline functions for x86_64 and SSE [BZ #17262] Message-ID: <20140812183008.GO9284@spoyarek.pnq.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ivHc2SZskddb40s2" Content-Disposition: inline User-Agent: Mutt/1.5.22.1-rc1 (2013-10-16) |
Commit Message
Siddhesh Poyarekar
Aug. 12, 2014, 6:30 p.m. UTC
Hi, Since: commit 409e00bd69b8d8dd74d7327085351d26769ea6fc Author: H.J. Lu <hjl.tools@gmail.com> Date: Wed Jan 29 07:51:41 2014 -0800 Disable x87 inline functions for SSE2 math When i386 and x86-64 mathinline.h was merged into a single mathinline.h, "gcc -m32" enables x87 inline functions on x86-64 even when -mfpmath=sse and SSE2 is enabled. It is a regression on x86-64. We should check __SSE2_MATH__ instead of __x86_64__ when disabling x87 inline functions. gcc-3.2 is unable to correctly compile x86_64 routines for llrint since it gets redefined. This is because gcc 3.2 does not set __SSE2_MATH__ for x86_64, thus exposing the duplicate definition. The correct fix ought to be to check for both __SSE2_MATH__ and __x86_64__ and enable those bits only when neither are defined. Tested fix with the reproducer for 409e00bd69b8d8dd74d7327085351d26769ea6fc as well as with gcc-3.2. OK for 2.20? Thanks, Siddhesh [BZ #17262] * sysdeps/x86/fpu/bits/mathinline.h: Check both __SSE2_MATH__ and __x86_64__ when disabling x87 inline functions.
Comments
On Wed, Aug 13, 2014 at 12:00:08AM +0530, Siddhesh Poyarekar wrote: > --- a/sysdeps/x86/fpu/bits/mathinline.h > +++ b/sysdeps/x86/fpu/bits/mathinline.h > @@ -384,7 +384,10 @@ __END_NAMESPACE_C99 > # endif > #endif > > -#ifndef __SSE2_MATH__ > +/* Disable x87 inlines when -fpmath=sse is passed and also when we're building > + on x86_64. Older gcc (gcc-3.2 for example) does not set __SSE2_MATH__ I'd use predefine or define instead of set, otherwise LGTM. Thanks for working on this. > + for x86_64. */ > +#if !defined __SSE2_MATH__ && !defined __x86_64__ > # if ((!defined __NO_MATH_INLINES || defined __LIBC_INTERNAL_MATH_INLINES) \ > && defined __OPTIMIZE__) > > @@ -970,4 +973,4 @@ __inline_mathcode2 (__ieee754_atan2, __y, __x, > return __value;) > # endif > > -#endif /* !__SSE2_MATH__ */ > +#endif /* !__SSE2_MATH__ && !__x86_64__ */ Jakub
On Tue, Aug 12, 2014 at 11:30 AM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote: > Hi, > > Since: > > commit 409e00bd69b8d8dd74d7327085351d26769ea6fc > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Wed Jan 29 07:51:41 2014 -0800 > > Disable x87 inline functions for SSE2 math > > When i386 and x86-64 mathinline.h was merged into a single mathinline.h, > "gcc -m32" enables x87 inline functions on x86-64 even when -mfpmath=sse > and SSE2 is enabled. It is a regression on x86-64. We should check > __SSE2_MATH__ instead of __x86_64__ when disabling x87 inline functions. > > > gcc-3.2 is unable to correctly compile x86_64 routines for llrint > since it gets redefined. This is because gcc 3.2 does not set > __SSE2_MATH__ for x86_64, thus exposing the duplicate definition. > > The correct fix ought to be to check for both __SSE2_MATH__ and > __x86_64__ and enable those bits only when neither are defined. > > Tested fix with the reproducer for > 409e00bd69b8d8dd74d7327085351d26769ea6fc as well as with gcc-3.2. > > OK for 2.20? > > Thanks, > Siddhesh > > [BZ #17262] > * sysdeps/x86/fpu/bits/mathinline.h: Check both __SSE2_MATH__ > and __x86_64__ when disabling x87 inline functions. > > diff --git a/sysdeps/x86/fpu/bits/mathinline.h b/sysdeps/x86/fpu/bits/mathinline.h > index 9c32e95..ee88b66 100644 > --- a/sysdeps/x86/fpu/bits/mathinline.h > +++ b/sysdeps/x86/fpu/bits/mathinline.h > @@ -384,7 +384,10 @@ __END_NAMESPACE_C99 > # endif > #endif > > -#ifndef __SSE2_MATH__ > +/* Disable x87 inlines when -fpmath=sse is passed and also when we're building > + on x86_64. Older gcc (gcc-3.2 for example) does not set __SSE2_MATH__ > + for x86_64. */ > +#if !defined __SSE2_MATH__ && !defined __x86_64__ > # if ((!defined __NO_MATH_INLINES || defined __LIBC_INTERNAL_MATH_INLINES) \ > && defined __OPTIMIZE__) > > @@ -970,4 +973,4 @@ __inline_mathcode2 (__ieee754_atan2, __y, __x, > return __value;) > # endif > > -#endif /* !__SSE2_MATH__ */ > +#endif /* !__SSE2_MATH__ && !__x86_64__ */ Will this patch get x86 inline functions with -fpmath=sse -m32 for GCC 3.2?
On Tue, Aug 12, 2014 at 11:48:44AM -0700, H.J. Lu wrote: > > --- a/sysdeps/x86/fpu/bits/mathinline.h > > +++ b/sysdeps/x86/fpu/bits/mathinline.h > > @@ -384,7 +384,10 @@ __END_NAMESPACE_C99 > > # endif > > #endif > > > > -#ifndef __SSE2_MATH__ > > +/* Disable x87 inlines when -fpmath=sse is passed and also when we're building > > + on x86_64. Older gcc (gcc-3.2 for example) does not set __SSE2_MATH__ > > + for x86_64. */ > > +#if !defined __SSE2_MATH__ && !defined __x86_64__ > > # if ((!defined __NO_MATH_INLINES || defined __LIBC_INTERNAL_MATH_INLINES) \ > > && defined __OPTIMIZE__) > > > > @@ -970,4 +973,4 @@ __inline_mathcode2 (__ieee754_atan2, __y, __x, > > return __value;) > > # endif > > > > -#endif /* !__SSE2_MATH__ */ > > +#endif /* !__SSE2_MATH__ && !__x86_64__ */ > > Will this patch get x86 inline functions with -fpmath=sse -m32 for > GCC 3.2? You'll get the i387 ones in that case, as GCC 3.2 and earlier just don't predefine anything you could use for that. Before this patch it didn't do that either. The bug in the current mathinline.h is that for gcc 3.2 and earlier on x86_64 you get errors on the mathinline.h, because you have multiple definitions of the inlines when __SSE2_MATH__ is not defined, but __x86_64__ is. Jakub
Looks right to me. It should go into 2.20 since it's a regression introduced since 2.19.
On 13/08/14 04:30, Siddhesh Poyarekar wrote:
> OK for 2.20?
Yes.
Allan
diff --git a/sysdeps/x86/fpu/bits/mathinline.h b/sysdeps/x86/fpu/bits/mathinline.h index 9c32e95..ee88b66 100644 --- a/sysdeps/x86/fpu/bits/mathinline.h +++ b/sysdeps/x86/fpu/bits/mathinline.h @@ -384,7 +384,10 @@ __END_NAMESPACE_C99 # endif #endif -#ifndef __SSE2_MATH__ +/* Disable x87 inlines when -fpmath=sse is passed and also when we're building + on x86_64. Older gcc (gcc-3.2 for example) does not set __SSE2_MATH__ + for x86_64. */ +#if !defined __SSE2_MATH__ && !defined __x86_64__ # if ((!defined __NO_MATH_INLINES || defined __LIBC_INTERNAL_MATH_INLINES) \ && defined __OPTIMIZE__) @@ -970,4 +973,4 @@ __inline_mathcode2 (__ieee754_atan2, __y, __x, return __value;) # endif -#endif /* !__SSE2_MATH__ */ +#endif /* !__SSE2_MATH__ && !__x86_64__ */