diff mbox

i386: Do not attempt to change stack alignment

Message ID 20180522081927.7608F416529F7@oldenburg.str.redhat.com
State Superseded
Headers

Commit Message

Florian Weimer May 22, 2018, 8:19 a.m. UTC 
  All supported GCC versions handle stack alignment as expected.

2018-05-22  Florian Weimer  <fweimer@redhat.com>

	i386: Do not attempt to change stack alignment.  All supported GCC
	versions handle stack alignment as expected.
	* sysdeps/i386/Makefile [$(subdir) == math] (sysdep-CFLAGS): Do
	not add -mpreferred-stack-boundary=4.
	[$(subdir) == csu] (sysdep-CFLAGS): Likewise.
	(stack-align-test-flags): Likewise.
	[$(subdir) == stdlib] (CFLAGS-exit.c, CFLAGS-cxa_finalize.c):
	Likewise.
	[$(subdir) == elf] (CFLAGS-dl-init.c, CFLAGS-dl-fini.c)
	(CFLAGS-dl-open.c, CFLAGS-dl-close.c, CFLAGS-dl-error.c): Likewise.
	[$(subdir) == dlfcn] (CFLAGS-dlopen.c, CFLAGS-dlopenold.c)
	(CFLAGS-dlclose.c, CFLAGS-dlerror.c): Likewise.
	* sysdeps/i386/nptl/Makefile [$(subdir) == nptl]
	(CFLAGS-pthread_create.c, CFLAGS-tst-align.c)
	(CFLAGS-tst-align2.c): Likewise.

Comments

Andreas Schwab May 22, 2018, 9:21 a.m. UTC | #1 
On Mai 22 2018, fweimer@redhat.com (Florian Weimer) wrote:

> All supported GCC versions handle stack alignment as expected.

Can you explain what is expected here?

Andreas.
Florian Weimer May 22, 2018, 9:26 a.m. UTC | #2 
On 05/22/2018 11:21 AM, Andreas Schwab wrote:
> On Mai 22 2018, fweimer@redhat.com (Florian Weimer) wrote:
> 
>> All supported GCC versions handle stack alignment as expected.
> 
> Can you explain what is expected here?

As required by the current i386 ABI.  Should I put that into the commit 
message?

It seems we used to have an optimization which avoided stack alignment 
for internal calls, but I doubt it was ever completely valid due to 
symbol interposition.  With present compilers, it would not work even 
for internal glibc use due to the scalar-to-vector pass and 
auto-vectorization (at -O3).

Thanks,
Florian
Andreas Schwab May 22, 2018, 9:33 a.m. UTC | #3 
On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:

> On 05/22/2018 11:21 AM, Andreas Schwab wrote:
>> On Mai 22 2018, fweimer@redhat.com (Florian Weimer) wrote:
>>
>>> All supported GCC versions handle stack alignment as expected.
>>
>> Can you explain what is expected here?
>
> As required by the current i386 ABI.

What is required, and how has the situation changed?

Andreas.
Florian Weimer May 22, 2018, 9:37 a.m. UTC | #4 
On 05/22/2018 11:33 AM, Andreas Schwab wrote:
> On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 05/22/2018 11:21 AM, Andreas Schwab wrote:
>>> On Mai 22 2018, fweimer@redhat.com (Florian Weimer) wrote:
>>>
>>>> All supported GCC versions handle stack alignment as expected.
>>>
>>> Can you explain what is expected here?
>>
>> As required by the current i386 ABI.
> 
> What is required, and how has the situation changed?

This was the stack alignment ABI change for the introduction of the SSE2 
ABI, so it's ancient history.

Thanks,
Florian
Andreas Schwab May 22, 2018, 9:54 a.m. UTC | #5 
On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:

> On 05/22/2018 11:33 AM, Andreas Schwab wrote:
>> On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> On 05/22/2018 11:21 AM, Andreas Schwab wrote:
>>>> On Mai 22 2018, fweimer@redhat.com (Florian Weimer) wrote:
>>>>
>>>>> All supported GCC versions handle stack alignment as expected.
>>>>
>>>> Can you explain what is expected here?
>>>
>>> As required by the current i386 ABI.
>>
>> What is required, and how has the situation changed?
>
> This was the stack alignment ABI change for the introduction of the SSE2
> ABI, so it's ancient history.

Are you sure?  Glibc used to use -mpreferred-stack-boundary=2, so I
don't see how that is related to GCC.

Andreas.
Florian Weimer May 22, 2018, 10:07 a.m. UTC | #6 
On 05/22/2018 11:54 AM, Andreas Schwab wrote:
> On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 05/22/2018 11:33 AM, Andreas Schwab wrote:
>>> On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> On 05/22/2018 11:21 AM, Andreas Schwab wrote:
>>>>> On Mai 22 2018, fweimer@redhat.com (Florian Weimer) wrote:
>>>>>
>>>>>> All supported GCC versions handle stack alignment as expected.
>>>>>
>>>>> Can you explain what is expected here?
>>>>
>>>> As required by the current i386 ABI.
>>>
>>> What is required, and how has the situation changed?
>>
>> This was the stack alignment ABI change for the introduction of the SSE2
>> ABI, so it's ancient history.
> 
> Are you sure?  Glibc used to use -mpreferred-stack-boundary=2, so I
> don't see how that is related to GCC.

Well, 4-byte alignment has been broken for a while now, and with STV, 
it's not just math code that is affected (even at -O2).

I don't really understand what you are after here. 
-mpreferred-stack-boundary=4 is the default, so the Makefile bits I 
removed are just a cleanup that was missed when the (rather broken) 
-mpreferred-stack-boundary=2 flag was removed.

Thanks,
Florian
Andreas Schwab May 22, 2018, 10:24 a.m. UTC | #7 
On Mai 22 2018, Florian Weimer <fweimer@redhat.com> wrote:

> I don't really understand what you are after
> here. -mpreferred-stack-boundary=4 is the default, so the Makefile bits I
> removed are just a cleanup that was missed when the (rather broken)
> -mpreferred-stack-boundary=2 flag was removed.

My issue is that this has nothing to do with GCC, but with our (former)
use of the option.  Your summary is misleading.

Andreas.
diff mbox

Patch

diff --git a/sysdeps/i386/Makefile b/sysdeps/i386/Makefile
index a1500454e5..1682394e76 100644
--- a/sysdeps/i386/Makefile
+++ b/sysdeps/i386/Makefile
@@ -19,46 +19,10 @@  CFLAGS-dl-load.c += -Wno-unused
 CFLAGS-dl-reloc.c += -Wno-unused
 endif
 
-# Most of the glibc routines don't ever call user defined callbacks
-# nor use any FPU or SSE* and as such don't need bigger %esp alignment
-# than 4 bytes.
-# Lots of routines in math will use FPU, so make math subdir an exception
-# here.
-# In gcc 4.6 (and maybe earlier?) giving -mpreferred-stack-boundary=2 is
-# an error, so don't try to reduce it here like we used to.  We still
-# explicit set -mpreferred-stack-boundary=4 the places where it matters,
-# in case an older compiler defaulted to 2.
-ifeq ($(subdir),math)
-sysdep-CFLAGS += -mpreferred-stack-boundary=4
-else
 ifeq ($(subdir),csu)
-sysdep-CFLAGS += -mpreferred-stack-boundary=4
 gen-as-const-headers += link-defines.sym
 else
-# Likewise, any function which calls user callbacks
-uses-callbacks += -mpreferred-stack-boundary=4
-# Likewise, any stack alignment tests
-stack-align-test-flags += -malign-double -mpreferred-stack-boundary=4
-endif
-endif
-
-# And a couple of other routines
-ifeq ($(subdir),stdlib)
-CFLAGS-exit.c += -mpreferred-stack-boundary=4
-CFLAGS-cxa_finalize.c += -mpreferred-stack-boundary=4
-endif
-ifeq ($(subdir),elf)
-CFLAGS-dl-init.c += -mpreferred-stack-boundary=4
-CFLAGS-dl-fini.c += -mpreferred-stack-boundary=4
-CFLAGS-dl-open.c += -mpreferred-stack-boundary=4
-CFLAGS-dl-close.c += -mpreferred-stack-boundary=4
-CFLAGS-dl-error.c += -mpreferred-stack-boundary=4
-endif
-ifeq ($(subdir),dlfcn)
-CFLAGS-dlopen.c += -mpreferred-stack-boundary=4
-CFLAGS-dlopenold.c += -mpreferred-stack-boundary=4
-CFLAGS-dlclose.c += -mpreferred-stack-boundary=4
-CFLAGS-dlerror.c += -mpreferred-stack-boundary=4
+stack-align-test-flags += -malign-double
 endif
 
 ifneq (,$(filter -mno-tls-direct-seg-refs,$(CFLAGS)))
diff --git a/sysdeps/i386/nptl/Makefile b/sysdeps/i386/nptl/Makefile
index 93a6d30b44..73024033ee 100644
--- a/sysdeps/i386/nptl/Makefile
+++ b/sysdeps/i386/nptl/Makefile
@@ -18,9 +18,3 @@ 
 ifeq ($(subdir),csu)
 gen-as-const-headers += tcb-offsets.sym
 endif
-
-ifeq ($(subdir),nptl)
-CFLAGS-pthread_create.c += -mpreferred-stack-boundary=4
-CFLAGS-tst-align.c += -mpreferred-stack-boundary=4
-CFLAGS-tst-align2.c += -mpreferred-stack-boundary=4
-endif