diff mbox

Don't build 32-bit libatomic with -march=i486 on x86-64

Message ID CAMe9rOr6mSP3Dv3JUXMBP0ehirUEimGXO_Zcw2avADXHeHQ3mQ@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu April 20, 2016, 2:45 p.m. UTC
On Wed, Apr 20, 2016 at 12:02 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>
>> That is why I submitted my patches.  Since -m32 passes -march=x86-64
>> to cc1 on x86-64,  we shouldn't pass -march=i486 to cc1.  It is undesirable
>> especially when --with-arch= is used.  I noticed the issue when 32-bit
>> libatomic/libgomp/libitm weren't optimized with -march=haswell when GCC
>> was configured with --with-arch=haswell
>
> OK then. IMO, following comment is more informative:
>
> # x86_64 compiler passes -march=x86_64 by default when building 32bit
> target libraries.
>
>>>>>>>> +       # Since 64-bit arch > i486, we can use the same -march= to build
>>>>>>>> +       # both 32-bit and 64-bit target libraries.
>
> OK with the above change.
>

This is the patch I checked in.  I also updated patches for libgomp:

https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01079.html

and libitm:

https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01080.html

Thanks.

Comments

Jakub Jelinek April 20, 2016, 2:54 p.m. UTC | #1
On Wed, Apr 20, 2016 at 07:45:44AM -0700, H.J. Lu wrote:
> On Wed, Apr 20, 2016 at 12:02 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> >>
> >> That is why I submitted my patches.  Since -m32 passes -march=x86-64
> >> to cc1 on x86-64,  we shouldn't pass -march=i486 to cc1.  It is undesirable
> >> especially when --with-arch= is used.  I noticed the issue when 32-bit
> >> libatomic/libgomp/libitm weren't optimized with -march=haswell when GCC
> >> was configured with --with-arch=haswell
> >
> > OK then. IMO, following comment is more informative:
> >
> > # x86_64 compiler passes -march=x86_64 by default when building 32bit
> > target libraries.
> >
> >>>>>>>> +       # Since 64-bit arch > i486, we can use the same -march= to build
> >>>>>>>> +       # both 32-bit and 64-bit target libraries.
> >
> > OK with the above change.
> >
> 
> This is the patch I checked in.  I also updated patches for libgomp:
> 
> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01079.html
> 
> and libitm:
> 
> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01080.html

This is wrong, see my other comment on the libgomp patch.

> 	PR target/70454
> 	* configure.tgt (XCFLAGS): Don't add -march=i486 to compile
> 	32-bit x86 target library on x86-64.
> ---
>  libatomic/configure.tgt | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
> index c5470d7..49233a4 100644
> --- a/libatomic/configure.tgt
> +++ b/libatomic/configure.tgt
> @@ -81,14 +81,8 @@ case "${target_cpu}" in
>  	try_ifunc=yes
>  	;;
>    x86_64)
> -	case " ${CC} ${CFLAGS} " in
> -	  *" -m32 "*)
> -	    XCFLAGS="${XCFLAGS} -march=i486 -mtune=generic"
> -	    XCFLAGS="${XCFLAGS} -fomit-frame-pointer"
> -	    ;;
> -	  *)
> -	    ;;
> -	esac
> +	# x86_64 compiler passes -march=x86_64 by default when building
> +	# 32bit target libraries.
>  	ARCH=x86
>  	# ??? Detect when -mcx16 is already enabled.
>  	try_ifunc=yes


	Jakub
H.J. Lu April 20, 2016, 2:57 p.m. UTC | #2
On Wed, Apr 20, 2016 at 7:54 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Apr 20, 2016 at 07:45:44AM -0700, H.J. Lu wrote:
>> On Wed, Apr 20, 2016 at 12:02 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >>
>> >> That is why I submitted my patches.  Since -m32 passes -march=x86-64
>> >> to cc1 on x86-64,  we shouldn't pass -march=i486 to cc1.  It is undesirable
>> >> especially when --with-arch= is used.  I noticed the issue when 32-bit
>> >> libatomic/libgomp/libitm weren't optimized with -march=haswell when GCC
>> >> was configured with --with-arch=haswell
>> >
>> > OK then. IMO, following comment is more informative:
>> >
>> > # x86_64 compiler passes -march=x86_64 by default when building 32bit
>> > target libraries.
>> >
>> >>>>>>>> +       # Since 64-bit arch > i486, we can use the same -march= to build
>> >>>>>>>> +       # both 32-bit and 64-bit target libraries.
>> >
>> > OK with the above change.
>> >
>>
>> This is the patch I checked in.  I also updated patches for libgomp:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01079.html
>>
>> and libitm:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01080.html
>
> This is wrong, see my other comment on the libgomp patch.
>

See my reply to your reply on the libgomp patch.
Bernd Schmidt April 25, 2016, 9:46 a.m. UTC | #3
On 04/20/2016 04:57 PM, H.J. Lu wrote:
> On Wed, Apr 20, 2016 at 7:54 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01080.html
>>
>> This is wrong, see my other comment on the libgomp patch.
>>
> See my reply to your reply on the libgomp patch.

Since Jakub has said it is wrong, please revert.


Bernd
Uros Bizjak April 25, 2016, 9:50 a.m. UTC | #4
On Mon, Apr 25, 2016 at 11:46 AM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 04/20/2016 04:57 PM, H.J. Lu wrote:
>>
>> On Wed, Apr 20, 2016 at 7:54 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>>
>>>> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01080.html
>>>
>>>
>>> This is wrong, see my other comment on the libgomp patch.
>>>
>> See my reply to your reply on the libgomp patch.
>
>
> Since Jakub has said it is wrong, please revert.

I agree.

Uros.
Uros Bizjak April 25, 2016, 9:52 a.m. UTC | #5
On Mon, Apr 25, 2016 at 11:50 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Apr 25, 2016 at 11:46 AM, Bernd Schmidt <bschmidt@redhat.com> wrote:
>> On 04/20/2016 04:57 PM, H.J. Lu wrote:
>>>
>>> On Wed, Apr 20, 2016 at 7:54 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>>>
>>>>> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01080.html
>>>>
>>>>
>>>> This is wrong, see my other comment on the libgomp patch.
>>>>
>>> See my reply to your reply on the libgomp patch.
>>
>>
>> Since Jakub has said it is wrong, please revert.
>
> I agree.

(sent the message too fast...)

These patches obviously need some more discussion.

Uros.
diff mbox

Patch

From bafad333cdf4125bf245e05d82df824ffb62c9d5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 30 Mar 2016 05:51:28 -0700
Subject: [PATCH 1/3] Don't build 32-bit libatomic with -march=i486 on x86-64

Gcc uses the same -march= for both -m32 and -m64 on x86-64 unless
--with-arch-32= is used.  There is no need for -march=i486 to compile
32-bit libatomic on x86-64.

	PR target/70454
	* configure.tgt (XCFLAGS): Don't add -march=i486 to compile
	32-bit x86 target library on x86-64.
---
 libatomic/configure.tgt | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index c5470d7..49233a4 100644
--- a/libatomic/configure.tgt
+++ b/libatomic/configure.tgt
@@ -81,14 +81,8 @@  case "${target_cpu}" in
 	try_ifunc=yes
 	;;
   x86_64)
-	case " ${CC} ${CFLAGS} " in
-	  *" -m32 "*)
-	    XCFLAGS="${XCFLAGS} -march=i486 -mtune=generic"
-	    XCFLAGS="${XCFLAGS} -fomit-frame-pointer"
-	    ;;
-	  *)
-	    ;;
-	esac
+	# x86_64 compiler passes -march=x86_64 by default when building
+	# 32bit target libraries.
 	ARCH=x86
 	# ??? Detect when -mcx16 is already enabled.
 	try_ifunc=yes
-- 
2.5.5