[1/3] configure: Allow specifying that the compiler can't rely on a aligned stack

diff mbox

The newly released Android x86 ABI assumes only 4-byte aligned stack
(similar to using -mstackrealign with gcc). This patch allows the user
to tell configure about this via --disable-aligned-stack, until someone
figures out a better automatic detection of this condition.
---
 configure |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Martin Storsjö <martin@martin.st> writes:

> The newly released Android x86 ABI assumes only 4-byte aligned stack

x86 or x86_64?

> (similar to using -mstackrealign with gcc). This patch allows the user
> to tell configure about this via --disable-aligned-stack, until someone
> figures out a better automatic detection of this condition.

Does the automatic realignment not work?

> ---
>  configure |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 28e3e25..de1d9a3 100755
> --- a/configure
> +++ b/configure
> @@ -236,6 +236,7 @@ Advanced options (experts only):
>    --enable-pic             build position-independent code
>    --malloc-prefix=PFX      prefix malloc and related names with PFX
>    --enable-sram            allow use of on-chip SRAM
> +  --disable-aligned-stack  don't assume the stack is aligned
>    --disable-symver         disable symbol versioning
>    --optflags               override optimization-related compiler flags
>
> @@ -1142,6 +1143,7 @@ CMDLINE_SELECT="
>      $ARCH_EXT_LIST
>      $CONFIG_LIST
>      $THREADS_LIST
> +    aligned_stack
>      asm
>      cross_compile
>      debug
> @@ -2630,7 +2632,7 @@ elif enabled mips; then
>
>  elif enabled ppc; then
>
> -    enable local_aligned_8 local_aligned_16
> +    disabled aligned_stack || enable local_aligned_8 local_aligned_16
>
>      check_asm dcbzl     '"dcbzl 0, %0" :: "r"(0)'
>      check_asm ibm_asm   '"add 0, 0, 0"'
> @@ -2668,7 +2670,7 @@ elif enabled sparc; then
>
>  elif enabled x86; then
>
> -    enable local_aligned_8 local_aligned_16
> +    disabled aligned_stack || enable local_aligned_8 local_aligned_16
>
>      # check whether EBP is available on x86
>      # As 'i' is stored on the stack, this program will crash
> -- 

It would be better to "enable aligned_stack" somewhere at the top and
add _deps variables for the local_aligned_* settings.  Seems more
future-proof.

On Sun, 17 Jul 2011, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
> 
> > The newly released Android x86 ABI assumes only 4-byte aligned stack
> 
> x86 or x86_64?

x86 32 bit (they call it 'x86' instead of i386/i686 as most others do).

> > (similar to using -mstackrealign with gcc). This patch allows the user
> > to tell configure about this via --disable-aligned-stack, until someone
> > figures out a better automatic detection of this condition.
> 
> Does the automatic realignment not work?

Pardon my ignorance, what would that be? Aligning variables with 
__attribute__((aligned (n))), as DECLARE_ALIGNED does? Yes, it works, 
afaik, but it uses up an extra register, which makes some of the inline 
assembly fail - see patch 2.

> > ---
> >  configure |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 28e3e25..de1d9a3 100755
> > --- a/configure
> > +++ b/configure
> > @@ -236,6 +236,7 @@ Advanced options (experts only):
> >    --enable-pic             build position-independent code
> >    --malloc-prefix=PFX      prefix malloc and related names with PFX
> >    --enable-sram            allow use of on-chip SRAM
> > +  --disable-aligned-stack  don't assume the stack is aligned
> >    --disable-symver         disable symbol versioning
> >    --optflags               override optimization-related compiler flags
> >
> > @@ -1142,6 +1143,7 @@ CMDLINE_SELECT="
> >      $ARCH_EXT_LIST
> >      $CONFIG_LIST
> >      $THREADS_LIST
> > +    aligned_stack
> >      asm
> >      cross_compile
> >      debug
> > @@ -2630,7 +2632,7 @@ elif enabled mips; then
> >
> >  elif enabled ppc; then
> >
> > -    enable local_aligned_8 local_aligned_16
> > +    disabled aligned_stack || enable local_aligned_8 local_aligned_16
> >
> >      check_asm dcbzl     '"dcbzl 0, %0" :: "r"(0)'
> >      check_asm ibm_asm   '"add 0, 0, 0"'
> > @@ -2668,7 +2670,7 @@ elif enabled sparc; then
> >
> >  elif enabled x86; then
> >
> > -    enable local_aligned_8 local_aligned_16
> > +    disabled aligned_stack || enable local_aligned_8 local_aligned_16
> >
> >      # check whether EBP is available on x86
> >      # As 'i' is stored on the stack, this program will crash
> > -- 
> 
> It would be better to "enable aligned_stack" somewhere at the top and
> add _deps variables for the local_aligned_* settings.  Seems more
> future-proof.

Yeah, that sounds sane. Didn't need any enable aligned_stack though, since 
it's got an _if_any line already, but the local_aligned_* needed _if 
instead of _deps in order to be automatically enabled.

// Martin

Martin Storsjö <martin@martin.st> writes:

> On Sun, 17 Jul 2011, Måns Rullgård wrote:
>
>> Martin Storsjö <martin@martin.st> writes:
>> 
>> > The newly released Android x86 ABI assumes only 4-byte aligned stack
>> 
>> x86 or x86_64?
>
> x86 32 bit (they call it 'x86' instead of i386/i686 as most others do).

People still use 32-bit x86?

>> > (similar to using -mstackrealign with gcc). This patch allows the user
>> > to tell configure about this via --disable-aligned-stack, until someone
>> > figures out a better automatic detection of this condition.
>> 
>> Does the automatic realignment not work?
>
> Pardon my ignorance, what would that be? Aligning variables with 
> __attribute__((aligned (n))), as DECLARE_ALIGNED does? Yes, it works, 
> afaik, but it uses up an extra register, which makes some of the inline 
> assembly fail - see patch 2.

I'm talking about __attribute__((force_align_arg_pointer)) as used on
all library entry points that might call functions needing aligned stack.

On Sun, 17 Jul 2011, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
> 
> > On Sun, 17 Jul 2011, Måns Rullgård wrote:
> >
> >> Martin Storsjö <martin@martin.st> writes:
> >> 
> >> > The newly released Android x86 ABI assumes only 4-byte aligned stack
> >> 
> >> x86 or x86_64?
> >
> > x86 32 bit (they call it 'x86' instead of i386/i686 as most others do).
> 
> People still use 32-bit x86?

Apparently...

> >> > (similar to using -mstackrealign with gcc). This patch allows the user
> >> > to tell configure about this via --disable-aligned-stack, until someone
> >> > figures out a better automatic detection of this condition.
> >> 
> >> Does the automatic realignment not work?
> >
> > Pardon my ignorance, what would that be? Aligning variables with 
> > __attribute__((aligned (n))), as DECLARE_ALIGNED does? Yes, it works, 
> > afaik, but it uses up an extra register, which makes some of the inline 
> > assembly fail - see patch 2.
> 
> I'm talking about __attribute__((force_align_arg_pointer)) as used on
> all library entry points that might call functions needing aligned stack.

Ah, yes, I didn't know we had such a thing in place. Yes, with that in 
place, libav compiles fine with -mincoming-stack-boundary=4 - then the 
DECLARE_ALIGNED for local variables doesn't seem to use up extra 
registers, and the two problematic inline assembly sections build fine. 
And the function prologue for functions marked with that attribute looks 
like it aligns the stack correctly.

So I guess this whole patchset can be dropped then.

// Martin