x86: Define LIBGCC2_UNWIND_ATTRIBUTE on ia32 [PR104781]

Message ID YieT0l9915XwHt7c@tucnak
State New
Headers
Series x86: Define LIBGCC2_UNWIND_ATTRIBUTE on ia32 [PR104781] |

Commit Message

Jakub Jelinek March 8, 2022, 5:35 p.m. UTC
  On Tue, Mar 08, 2022 at 08:09:25AM -0800, H.J. Lu wrote:
> > Ok.  So, what do you think about replacing the libgcc/ part of your patch
> > with that
> > /* __builtin_eh_return can't handle stack realignment, so disable SSE in
> >    32-bit libgcc functions that call it.  */
> > #ifndef __x86_64__
> > #define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> > #endif
> > ?
> 
> Yes, it should work.

So, how do we move on with this?
I can't self-approve my own patch, so can anyone please ack the following
provided it passes bootstraps/regtests ({x86_64,i686}-linux) that are
currently pending?

That can go in independently from your patch, and if it is committed,
your V3 patch with the libgcc/ hunks removed is preapproved for trunk.

2022-03-08  Jakub Jelinek  <jakub@redhat.com>

	PR target/104781
	* config/i386/i386.h (LIBGCC2_UNWIND_ATTRIBUTE): Define for ia32.



	Jakub
  

Comments

H.J. Lu March 8, 2022, 5:49 p.m. UTC | #1
On Tue, Mar 8, 2022 at 9:35 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Mar 08, 2022 at 08:09:25AM -0800, H.J. Lu wrote:
> > > Ok.  So, what do you think about replacing the libgcc/ part of your patch
> > > with that
> > > /* __builtin_eh_return can't handle stack realignment, so disable SSE in
> > >    32-bit libgcc functions that call it.  */
> > > #ifndef __x86_64__
> > > #define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> > > #endif
> > > ?
> >
> > Yes, it should work.
>
> So, how do we move on with this?
> I can't self-approve my own patch, so can anyone please ack the following
> provided it passes bootstraps/regtests ({x86_64,i686}-linux) that are
> currently pending?
>
> That can go in independently from your patch, and if it is committed,
> your V3 patch with the libgcc/ hunks removed is preapproved for trunk.
>
> 2022-03-08  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/104781
>         * config/i386/i386.h (LIBGCC2_UNWIND_ATTRIBUTE): Define for ia32.
>
> --- gcc/config/i386/i386.h.jj   2022-02-25 12:06:45.535493490 +0100
> +++ gcc/config/i386/i386.h      2022-03-08 11:20:43.207043370 +0100
> @@ -2848,6 +2848,12 @@ extern enum attr_cpu ix86_schedule;
>  #define NUM_X86_64_MS_CLOBBERED_REGS 12
>  #endif
>
> +/* __builtin_eh_return can't handle stack realignment, so disable SSE in
> +   32-bit libgcc functions that call it.  */
> +#ifndef __x86_64__
> +#define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> +#endif
> +
>  /*
>  Local variables:
>  version-control: t
>
>
>         Jakub
>

LGTM.

Thanks.
  
H.J. Lu March 8, 2022, 5:50 p.m. UTC | #2
On Tue, Mar 8, 2022 at 9:35 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Mar 08, 2022 at 08:09:25AM -0800, H.J. Lu wrote:
> > > Ok.  So, what do you think about replacing the libgcc/ part of your patch
> > > with that
> > > /* __builtin_eh_return can't handle stack realignment, so disable SSE in
> > >    32-bit libgcc functions that call it.  */
> > > #ifndef __x86_64__
> > > #define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> > > #endif
> > > ?
> >
> > Yes, it should work.
>
> So, how do we move on with this?
> I can't self-approve my own patch, so can anyone please ack the following
> provided it passes bootstraps/regtests ({x86_64,i686}-linux) that are
> currently pending?
>
> That can go in independently from your patch, and if it is committed,
> your V3 patch with the libgcc/ hunks removed is preapproved for trunk.

I am checking in this:

https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591392.html

Thanks.

> 2022-03-08  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/104781
>         * config/i386/i386.h (LIBGCC2_UNWIND_ATTRIBUTE): Define for ia32.
>
> --- gcc/config/i386/i386.h.jj   2022-02-25 12:06:45.535493490 +0100
> +++ gcc/config/i386/i386.h      2022-03-08 11:20:43.207043370 +0100
> @@ -2848,6 +2848,12 @@ extern enum attr_cpu ix86_schedule;
>  #define NUM_X86_64_MS_CLOBBERED_REGS 12
>  #endif
>
> +/* __builtin_eh_return can't handle stack realignment, so disable SSE in
> +   32-bit libgcc functions that call it.  */
> +#ifndef __x86_64__
> +#define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> +#endif
> +
>  /*
>  Local variables:
>  version-control: t
>
>
>         Jakub
>
  
Richard Biener March 9, 2022, 7:18 a.m. UTC | #3
On Tue, 8 Mar 2022, H.J. Lu wrote:

> On Tue, Mar 8, 2022 at 9:35 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Tue, Mar 08, 2022 at 08:09:25AM -0800, H.J. Lu wrote:
> > > > Ok.  So, what do you think about replacing the libgcc/ part of your patch
> > > > with that
> > > > /* __builtin_eh_return can't handle stack realignment, so disable SSE in
> > > >    32-bit libgcc functions that call it.  */
> > > > #ifndef __x86_64__
> > > > #define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> > > > #endif
> > > > ?
> > >
> > > Yes, it should work.
> >
> > So, how do we move on with this?
> > I can't self-approve my own patch, so can anyone please ack the following
> > provided it passes bootstraps/regtests ({x86_64,i686}-linux) that are
> > currently pending?
> >
> > That can go in independently from your patch, and if it is committed,
> > your V3 patch with the libgcc/ hunks removed is preapproved for trunk.
> >
> > 2022-03-08  Jakub Jelinek  <jakub@redhat.com>
> >
> >         PR target/104781
> >         * config/i386/i386.h (LIBGCC2_UNWIND_ATTRIBUTE): Define for ia32.
> >
> > --- gcc/config/i386/i386.h.jj   2022-02-25 12:06:45.535493490 +0100
> > +++ gcc/config/i386/i386.h      2022-03-08 11:20:43.207043370 +0100
> > @@ -2848,6 +2848,12 @@ extern enum attr_cpu ix86_schedule;
> >  #define NUM_X86_64_MS_CLOBBERED_REGS 12
> >  #endif
> >
> > +/* __builtin_eh_return can't handle stack realignment, so disable SSE in
> > +   32-bit libgcc functions that call it.  */
> > +#ifndef __x86_64__
> > +#define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
> > +#endif
> > +
> >  /*
> >  Local variables:
> >  version-control: t
> >
> >
> >         Jakub
> >
> 
> LGTM.

I wonder if this is a good case for general-regs-only instead?  At
least no-sse cannot be functionally equivalent (since then we would
not have needed general-regs-only ...).

Richard.
  

Patch

--- gcc/config/i386/i386.h.jj	2022-02-25 12:06:45.535493490 +0100
+++ gcc/config/i386/i386.h	2022-03-08 11:20:43.207043370 +0100
@@ -2848,6 +2848,12 @@  extern enum attr_cpu ix86_schedule;
 #define NUM_X86_64_MS_CLOBBERED_REGS 12
 #endif
 
+/* __builtin_eh_return can't handle stack realignment, so disable SSE in
+   32-bit libgcc functions that call it.  */
+#ifndef __x86_64__
+#define LIBGCC2_UNWIND_ATTRIBUTE __attribute__((target ("no-sse")))
+#endif
+
 /*
 Local variables:
 version-control: t