powerpc-linux-gnu bootstrap fail

powerpc-linux-gnu bootstrap fail

[Franz Sirl]

Hi,

shouldn't the mainline stabilize? Not only that my 3.1pre testing is 
heavily blocked by serious bugs like PR 5693 and gcc.dg/20020103-1.c, now 
it doesn't even bootstrap anymore :-(.

Franz.

stage1/xgcc -Bstage1/ -B/home/fsirl/gnubin/ppc-linux/bin/ -c -DIN_GCC    -g 
-O2 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
-Wtraditional -pedantic -Wno-long-long  -DHAVE_CONFIG_H    -I. -I. 
-I/home/fsirl/cvsx/gccm/gcc -I/home/fsirl/cvsx/gccm/gcc/. 
-I/home/fsirl/cvsx/gccm/gcc/config -I/home/fsirl/cvsx/gccm/gcc/../include 
/home/fsirl/cvsx/gccm/gcc/dwarfout.c -o dwarfout.o
/home/fsirl/cvsx/gccm/gcc/dwarfout.c: In function `output_compile_unit_die':
/home/fsirl/cvsx/gccm/gcc/dwarfout.c:4177: Internal compiler error in 
simplify_unary_operation, at simplify-rtx.c:534
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://www.gnu.org/software/gcc/bugs.html> for instructions.

Re: powerpc-linux-gnu bootstrap fail

[David Edelsohn]

	Is this caused by Alexandre's patch from yesterday?  Is it fixed
by his rtlanal.c patch?

http://gcc.gnu.org/ml/gcc-patches/2002-02/msg01464.html

David

Re: powerpc-linux-gnu bootstrap fail

[Franz Sirl]

At 17:27 21.02.2002, David Edelsohn wrote:
>         Is this caused by Alexandre's patch from yesterday?  Is it fixed
>by his rtlanal.c patch?
>
>http://gcc.gnu.org/ml/gcc-patches/2002-02/msg01464.html

Yeah, this patch seems to fix it, at least bootstrap is in stage2 now.

Franz.

Re: powerpc-linux-gnu bootstrap fail

[Daniel Egger]

Am Don, 2002-02-21 um 18.56 schrieb Franz Sirl:

> Yeah, this patch seems to fix it, at least bootstrap is in stage2 now.

It worked perfectly for me yesterday, the day before and now. It just
finished bootstrapping.
 
-- 
Servus,
       Daniel

Re: Altivec + 16 byte alignment

[John David Anglin]

> I have a problem in that altivec requires all inputs to be 16 byte
> aligned, so I am placing my vectors in a structure like this:
> 
> struct vector {
>         float x,y,z,w;
> }__attribute__((aligned(16)));

We had a similar problem with respect to the alignment of data for
the ldcw instruction on the PA which needs 16-byte alignment on PA 1.X.
We have found an alternative solution.  However, the patch below
will allow the aligned attribute to be respected for stack locals
up to PREFERRED_STACK_BOUNDARY.  Note however, that alignment of
stack locals is only guaranteed up to STACK_BOUNDARY.  The default
alignment for structs on the stack is BIGGEST_ALIGNMENT, which I
presume is set 64 (8 bytes).

It may well be with altvec you really want BIGGEST_ALIGNMENT set to
256, but the change would affect the ABI and alignment of long doubles.
The i386 code provides an example of what needs doing if there are
types such as long doubles that don't require natural alignment.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2003-02-11  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* function.c (assign_stack_temp_for_type): Allow user to increase
	alignment of stack locals up to PREFERRED_STACK_BOUNDARY.

Index: function.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/function.c,v
retrieving revision 1.388
diff -u -3 -p -r1.388 function.c
--- function.c	27 Nov 2002 02:29:12 -0000	1.388
+++ function.c	6 Dec 2002 23:07:56 -0000
@@ -650,6 +650,12 @@ assign_stack_temp_for_type (mode, size, 
   else
     align = GET_MODE_ALIGNMENT (mode);
 
+  /* Allow the user to increase the default alignment with the aligned
+     attribute but bound the range.  */
+  if (type && TYPE_USER_ALIGN (type))
+    align = MIN (MAX (TYPE_ALIGN (type), align),
+		 MAX (PREFERRED_STACK_BOUNDARY, align));
+
   if (! type)
     type = (*lang_hooks.types.type_for_mode) (mode, 0);
 

Re: Altivec + 16 byte alignment

[Franz Sirl]

At 18:25 11.02.2003, John David Anglin wrote:
> > I have a problem in that altivec requires all inputs to be 16 byte
> > aligned, so I am placing my vectors in a structure like this:
> >
> > struct vector {
> >         float x,y,z,w;
> > }__attribute__((aligned(16)));
>
>We had a similar problem with respect to the alignment of data for
>the ldcw instruction on the PA which needs 16-byte alignment on PA 1.X.
>We have found an alternative solution.  However, the patch below
>will allow the aligned attribute to be respected for stack locals
>up to PREFERRED_STACK_BOUNDARY.  Note however, that alignment of
>stack locals is only guaranteed up to STACK_BOUNDARY.  The default
>alignment for structs on the stack is BIGGEST_ALIGNMENT, which I
>presume is set 64 (8 bytes).
>
>It may well be with altvec you really want BIGGEST_ALIGNMENT set to
>256, but the change would affect the ABI and alignment of long doubles.
>The i386 code provides an example of what needs doing if there are
>types such as long doubles that don't require natural alignment.

BIGGEST_ALIGNMENT is 128, but STACK_BOUNDARY is 64 unless you give 
-mabi=altivec (in 3.2+). Now, if we would increase STACK_BOUNDARY to be 
always 128 on powerpc-linux-gnu, would this affect binary compatibility at 
all? I can't think of a situation where this would happen, but maybe I'm wrong?
If changing STACK_BOUNDARY doesn't affect binary compatibility, I will 
submit a patch to set it to 128 in rs6000/linux.h.

Franz.

Re: Altivec + 16 byte alignment

[David Edelsohn]

>>>>> Franz Sirl writes:

Franz> BIGGEST_ALIGNMENT is 128, but STACK_BOUNDARY is 64 unless you give 
Franz> -mabi=altivec (in 3.2+). Now, if we would increase STACK_BOUNDARY to be 
Franz> always 128 on powerpc-linux-gnu, would this affect binary compatibility at 
Franz> all? I can't think of a situation where this would happen, but maybe I'm wrong?
Franz> If changing STACK_BOUNDARY doesn't affect binary compatibility, I will 
Franz> submit a patch to set it to 128 in rs6000/linux.h.

	The patch will be wrong and not accepted.

David

Re: Altivec + 16 byte alignment

[Franz Sirl]

At 19:32 11.02.2003, David Edelsohn wrote:
> >>>>> Franz Sirl writes:
>
>Franz> BIGGEST_ALIGNMENT is 128, but STACK_BOUNDARY is 64 unless you give
>Franz> -mabi=altivec (in 3.2+). Now, if we would increase STACK_BOUNDARY 
>to be
>Franz> always 128 on powerpc-linux-gnu, would this affect binary 
>compatibility at
>Franz> all? I can't think of a situation where this would happen, but 
>maybe I'm wrong?
>Franz> If changing STACK_BOUNDARY doesn't affect binary compatibility, I will
>Franz> submit a patch to set it to 128 in rs6000/linux.h.
>
>         The patch will be wrong and not accepted.

Uhm, wrong in which way? Would you mind to explain this a bit?

Franz.

Re: Altivec + 16 byte alignment

[Spundun Bhatt]

On Tue, 2003-02-11 at 10:57, Franz Sirl wrote:
> At 19:32 11.02.2003, David Edelsohn wrote:
> > >>>>> Franz Sirl writes:
> >
> >Franz> If changing STACK_BOUNDARY doesn't affect binary compatibility, I will
> >Franz> submit a patch to set it to 128 in rs6000/linux.h.
> >
> >         The patch will be wrong and not accepted.
> 
> Uhm, wrong in which way? Would you mind to explain this a bit?
> 
With all the discussion going on... I didnt see any replies to this
question... (I might have missed it... the threading in evolution for
this thread is skrewed up). Can somebody explain why this is wrong? To
me it looks pretty straight forward to simply change STACK_BOUNDARY to
128. Not that I care though.
Hope this helps
Spundun
> Franz.

Re: Altivec + 16 byte alignment

[Spundun Bhatt]

On Wed, 2003-02-12 at 09:26, Gianni Tedesco wrote:
> On Tue, 2003-02-11 at 22:33, Spundun Bhatt wrote:
> > On Tue, 2003-02-11 at 10:57, Franz Sirl wrote:
> > > At 19:32 11.02.2003, David Edelsohn wrote:
> > > > >>>>> Franz Sirl writes:
> > > >
> > > >Franz> If changing STACK_BOUNDARY doesn't affect binary compatibility, I will
> > > >Franz> submit a patch to set it to 128 in rs6000/linux.h.
> > > >
> > > >         The patch will be wrong and not accepted.
> > > 
> > > Uhm, wrong in which way? Would you mind to explain this a bit?
> 
> I think because the efficient way to do it is for the caller to always
> keep the stack aligned. eg: initial stack pointer is aligned to 16 bytes
> and all functions make sure their stack frames align to 16 also.
> 
> If you have functions with smaller aligned stack and then they call the
> altivec functions then the altivec function would have to look at the
> stack pointer and align it by hand, which would require a conditional
> branch, argh.
> 
Ofcourse you are right. This means that this trick wont work if used in
a library code if the library is used with an older gcc compiler which
doesnt support this bigger alignement. Also wont work if used for
callbacks with the library using the callbacks not suppoirting this
bigger alignement... and may be many other such instances. 
While this does say that there will be a binary incompatibility between
gcc versions... this incompatibility will only be for this wider
alignement issues. This doesnt in *any* way seems to break backward
compatibility. In general whenever a new feature is added to a software,
the new feature is not supposed to work in the older versions but the
backward compatibility needs to be preserved.. right? 
So I think Franz' question is still up in the air.... Unless ofcourse, I
didnt get your point
Thanx 
Spundun
> I guess...
> 
> HTH.

Re: Altivec + 16 byte alignment

[Richard Henderson]

On Tue, Feb 11, 2003 at 07:29:37PM +0100, Franz Sirl wrote:
> Now, if we would increase STACK_BOUNDARY to be 
> always 128 on powerpc-linux-gnu, would this affect binary compatibility at 
> all?

Very much so.


r~

Re: Altivec + 16 byte alignment

[Franz Sirl]

On Wednesday 12 February 2003 01:42, Richard Henderson wrote:
> On Tue, Feb 11, 2003 at 07:29:37PM +0100, Franz Sirl wrote:
> > Now, if we would increase STACK_BOUNDARY to be
> > always 128 on powerpc-linux-gnu, would this affect binary compatibility
> > at all?
>
> Very much so.

I'm feeling dumb, but can you give an example? AFAICS STACK_BOUNDARY only 
affects the alignment of objects on the stack, argument aligning is 
independent from it and the stack pointer is kept aligned (to 256 bits) by 
independent code as well, so how can these objects passed between functions 
without passing pointers to the objects as well?

Franz.

Re: Altivec + 16 byte alignment

[Richard Henderson]

On Wed, Feb 12, 2003 at 10:07:54PM +0100, Franz Sirl wrote:
> ... and the stack pointer is kept aligned (to 256 bits) by 
> independent code as well

Is this 256 bits ABI mandated?  I.e. will a non-gcc ppc
compiler maintain this alignment as well?

If yes, then STACK_ALIGNMENT is set too low.  You should
advertise what you have.  If no, then calling between two
functions compiled with different compilers will not 
preserve the 256 bit alignment you claim.

There are passes in gcc, particularly combine, that look
at the advertised alignment for a pointer and know that
the low bits are zero, and perform optimizations based on
this.  If the alignment doesn't exist, the optimizations
produce incorrect results.


r~

Re: Altivec + 16 byte alignment

[Franz Sirl]

On Wednesday 12 February 2003 22:46, Richard Henderson wrote:
> On Wed, Feb 12, 2003 at 10:07:54PM +0100, Franz Sirl wrote:
> > ... and the stack pointer is kept aligned (to 256 bits) by
> > independent code as well
>
> Is this 256 bits ABI mandated?  I.e. will a non-gcc ppc
> compiler maintain this alignment as well?

Yep, ABI mandated, 256 bits for SYSV, 128 bits for EABI.

> If yes, then STACK_ALIGNMENT is set too low.  You should
> advertise what you have.  If no, then calling between two
> functions compiled with different compilers will not
> preserve the 256 bit alignment you claim.

STACK_ALIGNMENT? I can't find that. Closest I can find is MIPS_STACK_ALIGN. 
Did you think of some other macro? Sounds promising though.

Maybe part of the confusion stems from the overloaded meaning of 
STACK_BOUNDARY, sometimes it means "alignment needed for the stack pointer", 
sometimes "maximum alignment of objects in the frame" or similar it seems.

Hmm, do we have a function like get_frame_max_object_align()? Then each 
backend could adjust alignment of the frame vs. the alignment of the 
stackpointer as required.

> There are passes in gcc, particularly combine, that look
> at the advertised alignment for a pointer and know that
> the low bits are zero, and perform optimizations based on
> this.  If the alignment doesn't exist, the optimizations
> produce incorrect results.

But this certainly isn't a problem if we _increase_ STACK_BOUNDARY like I 
suggested, or?

Franz.

Re: Altivec + 16 byte alignment

[Gabriel Paubert]

On Wed, Feb 12, 2003 at 11:26:48PM +0100, Franz Sirl wrote:
> On Wednesday 12 February 2003 22:46, Richard Henderson wrote:
> > On Wed, Feb 12, 2003 at 10:07:54PM +0100, Franz Sirl wrote:
> > > ... and the stack pointer is kept aligned (to 256 bits) by
> > > independent code as well
> >
> > Is this 256 bits ABI mandated?  I.e. will a non-gcc ppc
> > compiler maintain this alignment as well?
> 
> Yep, ABI mandated, 256 bits for SYSV, 128 bits for EABI.

I believe that they are actually half of this, at least on 32 bit
targets. Alignment constraints of 128 and 64 appear quite frequently 
in the config/rs6000 files.

	Regards,
	Gabriel

Re: Altivec + 16 byte alignment

[John David Anglin]

> If yes, then STACK_ALIGNMENT is set too low.  You should
> advertise what you have.  If no, then calling between two
> functions compiled with different compilers will not 
> preserve the 256 bit alignment you claim.

You mean STACK_BOUNDARY?

> There are passes in gcc, particularly combine, that look
> at the advertised alignment for a pointer and know that
> the low bits are zero, and perform optimizations based on
> this.  If the alignment doesn't exist, the optimizations
> produce incorrect results.

I assume that it's actually the value specified by
PREFERRED_STACK_BOUNDARY stack boundary that is used for
the above.

Currently on the PA 32-bit ports, STACK_BOUNDARY
is 64 (same as BIGGEST_ALIGNMENT and PARM_BOUNDARY) but
PREFERRED_STACK_BOUNDARY is 512.  In part, this is because
we must define a non-zero STARTING_FRAME_OFFSET and the
start of the locals must lie on a STACK_BOUNDARY.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: Altivec + 16 byte alignment

[Richard Henderson]

On Wed, Feb 12, 2003 at 05:31:20PM -0500, John David Anglin wrote:
> You mean STACK_BOUNDARY?

Yes, sorry.

> > There are passes in gcc, particularly combine, that look
> > at the advertised alignment for a pointer and know that
> > the low bits are zero, and perform optimizations based on
> > this.  If the alignment doesn't exist, the optimizations
> > produce incorrect results.
> 
> I assume that it's actually the value specified by
> PREFERRED_STACK_BOUNDARY stack boundary that is used for
> the above.

No, that would be wrong.

PREFERRED_STACK_BOUNDARY is a suggestion.  The theory is that
*if* every object file is compiled with this, that we'll get
some speedup at runtime, due to fewer misalignment fixups.

But it is *not* ABI mandated, and you have to assume that 
someone somewhere may not honor this.

> Currently on the PA 32-bit ports, STACK_BOUNDARY
> is 64 (same as BIGGEST_ALIGNMENT and PARM_BOUNDARY) but
> PREFERRED_STACK_BOUNDARY is 512.  In part, this is because
> we must define a non-zero STARTING_FRAME_OFFSET and the
> start of the locals must lie on a STACK_BOUNDARY.

True, but it's also assumed that they lie on a PREFERRED_STACK_BOUNDARY,
are they not?.  So I don't see that you're helping much?


r~

Re: Altivec + 16 byte alignment

[John David Anglin]

> PREFERRED_STACK_BOUNDARY is a suggestion.  The theory is that
> *if* every object file is compiled with this, that we'll get
> some speedup at runtime, due to fewer misalignment fixups.
> 
> But it is *not* ABI mandated, and you have to assume that 
> someone somewhere may not honor this.

Yes, it is.  It's written in every runtime document that I have looked
at.  HP-UX doesn't perform misalignment fixups, although the linux port
does.  I guess you are saying that STACK_BOUNDARY really should be 512,
although I have the feeling that that's incompatible with the alignment
used for the outgoing argument block.  Can you describe under what
circumstances if any gcc will not be able to align to the preferred
stack boundary?

Looking at the documentation, it says that STACK_BOUNDARY should be set
to the *minimum* alignment enforced by hardware for the stack pointer
on the machine.  It doesn't say the ABI mandated value.  64-bit alignment
of the stack is sufficient for any type, except for lock semaphores.
If the PREFERRED_STACK_BOUNDARY doesn't break down, it might be
possible to set STACK_BOUNDARY to 32 (i.e., the same as PARM_BOUNDARY).

When STACK_BOUNDARY is set to 64, there is one fill word in the frame
as a result of the starting frame offset and the need to align it to a
STACK_BOUNDARY.  If STACK_BOUNDARY were set to 512, we would have 15
fill words.

> > Currently on the PA 32-bit ports, STACK_BOUNDARY
> > is 64 (same as BIGGEST_ALIGNMENT and PARM_BOUNDARY) but
> > PREFERRED_STACK_BOUNDARY is 512.  In part, this is because
> > we must define a non-zero STARTING_FRAME_OFFSET and the
> > start of the locals must lie on a STACK_BOUNDARY.
> 
> True, but it's also assumed that they lie on a PREFERRED_STACK_BOUNDARY,
> are they not?.  So I don't see that you're helping much?

I am certain that STARTING_FRAME_OFFSET must lie on a STACK_BOUNDARY.
Otherwise, you never get a zero frame in a leaf function.  I believe this
is because of the alignment specified in init_emit for STACK_POINTER_REGNUM.
This requirement is independent of any alignment requirement for locals.

PREFERRED_STACK_BOUNDARY sets the frame alignment and an upper limit on
the alignment used in assign_stack_local_1.  There doesn't seem to be a
similar limit in assign_stack_temp_for_type.  As far as I can tell,
STARTING_FRAME_OFFSET doesn't need to be a multiple PREFERRED_STACK_BOUNDARY.
The allocation code will simply pad to the alignment needed for the first
local.

PREFERRED_STACK_BOUNDARY is set to the alignment mandated by the ABI
for the stack pointer.  The 64-byte alignment of the frame was
mandated so that compilers could know when data structures allocated
on the stack are cache aligned.  However, we don't get much benefit
from this using GCC.

It would help if user code could align stack data on a cache line for
performance or other purposes.  I believe the patch that I sent early in
this thread, <http://gcc.gnu.org/ml/gcc/2003-02/msg00656.html>, allows
a user to do this.  It also allows using the low bits as tag bits, as
suggested by Fergus Henderson.  However, it doesn't provide the suggested
error checking.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: Altivec + 16 byte alignment

[Richard Henderson]

On Thu, Feb 13, 2003 at 01:14:30AM -0500, John David Anglin wrote:
> > PREFERRED_STACK_BOUNDARY is a suggestion.  The theory is that
> > *if* every object file is compiled with this, that we'll get
> > some speedup at runtime, due to fewer misalignment fixups.
> > 
> > But it is *not* ABI mandated, and you have to assume that 
> > someone somewhere may not honor this.
> 
> Yes, it is.

No.  That is not the purpose of PREFERRED_STACK_BOUNDARY.
The ABI mandated quantity is STACK_BOUNDARY.

> I guess you are saying that STACK_BOUNDARY really should be 512...

If that's what the ABI mandates, yes.

> Can you describe under what
> circumstances if any gcc will not be able to align to the preferred
> stack boundary?

None.  That's not the point.

> Looking at the documentation, it says that STACK_BOUNDARY should be set
> to the *minimum* alignment enforced by hardware for the stack pointer
> on the machine.  It doesn't say the ABI mandated value.

Then the docs need updating.

> When STACK_BOUNDARY is set to 64, there is one fill word in the frame
> as a result of the starting frame offset and the need to align it to a
> STACK_BOUNDARY.  If STACK_BOUNDARY were set to 512, we would have 15
> fill words.

Eh?  Is the stack pointer aligned or not?



r~

Re: Altivec + 16 byte alignment

[John David Anglin]

> > When STACK_BOUNDARY is set to 64, there is one fill word in the frame
> > as a result of the starting frame offset and the need to align it to a
> > STACK_BOUNDARY.  If STACK_BOUNDARY were set to 512, we would have 15
> > fill words.
> 
> Eh?  Is the stack pointer aligned or not?

The comment for STACK_BOUNDARY says "GCC for the PA always rounds its
stacks to a 8 * STACK_BOUNDARY boundary, but that happens late in the
compilation process."  The final size and rounding is determined during
reload.  This calculation is done in compute_frame_size.  However, the
value returned is always rounded no matter when it is called.  We add
in STARTING_FRAME_OFFSET if the frame pointer is needed or the size
returned by get_frame_size () is non-zero.  We also add in space for
the outgoing args, register saves, etc.

Hmmm, I see a small bug.  The rounding in compute_frame_size incorrectly
uses STACK_BOUNDARY.  Based on our current methodology, it should be
using PREFERRED_STACK_BOUNDARY.  However, this isn't the answer to your
question.

I will investigate to see why the above occurs.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

[PATCH]: Stack alignment

[John David Anglin]

> > When STACK_BOUNDARY is set to 64, there is one fill word in the frame
> > as a result of the starting frame offset and the need to align it to a
> > STACK_BOUNDARY.  If STACK_BOUNDARY were set to 512, we would have 15
> > fill words.
> 
> Eh?  Is the stack pointer aligned or not?

This turned out to be totally wrong.  Here is the actual problem.

Reload aligns the stack in each iteration.  It uses a default alignment
value of BIGGEST_ALIGNMENT.  If STARTING_FRAME_OFFSET % BIGGEST_ALIGNMENT
is not zero, the rounding in assign_stack_temp_for_type will cause the
frame size to be non-zero.  As a result, you will never have a non-empty
frame and code for doing the stack adjustments needs to be emitted in
the prologue and epilogue.

The enclosed patch changes assign_stack_temp_for_type to only do the
reload alignment when the current frame size is not zero.  With this
fix, I can save eight bytes in the stack offset on the 64-bit PA ports.

It has been tested with no regressions on hppa2.0w-hp-hpux11.11 and
hppa64-hp-hpux11.11.

Ok for trunk and 3.3?  The reason for installing on 3.3 is that I
would like to revert the change that I made to STARTING_FRAME_OFFSET
on the 64-bit ports when I found that we never had a non-empty frame.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2003-02-17  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* function.c (assign_stack_temp_for_type): Don't create a non-zero
	frame when doing a stack alignment if the current frame size is zero.

Index: function.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/function.c,v
retrieving revision 1.399
diff -u -3 -p -r1.399 function.c
--- function.c	1 Feb 2003 18:59:44 -0000	1.399
+++ function.c	17 Feb 2003 17:06:06 -0000
@@ -564,16 +564,28 @@ assign_stack_local_1 (mode, size, align,
   frame_off = STARTING_FRAME_OFFSET % frame_alignment;
   frame_phase = frame_off ? frame_alignment - frame_off : 0;
 
-  /* Round frame offset to that alignment.
-     We must be careful here, since FRAME_OFFSET might be negative and
-     division with a negative dividend isn't as well defined as we might
-     like.  So we instead assume that ALIGNMENT is a power of two and
-     use logical operations which are unambiguous.  */
+  /* Round the frame offset to the specified alignment unless the
+     frame size is zero and this is a frame alignment request.  When
+     STARTING_FRAME_OFFSET is non-zero, we don't want a frame alignment
+     request to create a frame if one doesn't already exist.  */
+  if (function->x_frame_offset
+      || mode != BLKmode
+      || size != 0)
+    {
+      /*  We must be careful here, since FRAME_OFFSET might be negative and
+	  division with a negative dividend isn't as well defined as we might
+	  like.  So we instead assume that ALIGNMENT is a power of two and
+	  use logical operations which are unambiguous.  */
 #ifdef FRAME_GROWS_DOWNWARD
-  function->x_frame_offset = FLOOR_ROUND (function->x_frame_offset - frame_phase, alignment) + frame_phase;
+      function->x_frame_offset
+	= (FLOOR_ROUND (function->x_frame_offset - frame_phase, alignment)
+	   + frame_phase);
 #else
-  function->x_frame_offset = CEIL_ROUND (function->x_frame_offset - frame_phase, alignment) + frame_phase;
+      function->x_frame_offset
+	= (CEIL_ROUND (function->x_frame_offset - frame_phase, alignment)
+	   + frame_phase);
 #endif
+    }
 
   /* On a big-endian machine, if we are allocating more space than we will use,
      use the least significant bytes of those that are allocated.  */

Re: [PATCH]: Stack alignment

[Geoff Keating]

"John David Anglin" <dave@hiauly1.hia.nrc.ca> writes:

> > > When STACK_BOUNDARY is set to 64, there is one fill word in the frame
> > > as a result of the starting frame offset and the need to align it to a
> > > STACK_BOUNDARY.  If STACK_BOUNDARY were set to 512, we would have 15
> > > fill words.
> > 
> > Eh?  Is the stack pointer aligned or not?
> 
> This turned out to be totally wrong.  Here is the actual problem.
> 
> Reload aligns the stack in each iteration.  It uses a default alignment
> value of BIGGEST_ALIGNMENT.  If STARTING_FRAME_OFFSET % BIGGEST_ALIGNMENT
> is not zero, the rounding in assign_stack_temp_for_type will cause the
> frame size to be non-zero.  As a result, you will never have a non-empty
> frame and code for doing the stack adjustments needs to be emitted in
> the prologue and epilogue.

Right.  This is because you need to keep the stack aligned, so you can
never have an empty frame.  Which brings us back to a variant of the
previous poster's question: is the stack pointer supposed to be
aligned, or not?

-- 
- Geoffrey Keating <geoffk@geoffk.org>

Re: [PATCH]: Stack alignment

[John David Anglin]

> > Reload aligns the stack in each iteration.  It uses a default alignment
> > value of BIGGEST_ALIGNMENT.  If STARTING_FRAME_OFFSET % BIGGEST_ALIGNMENT
> > is not zero, the rounding in assign_stack_temp_for_type will cause the
> > frame size to be non-zero.  As a result, you will never have a non-empty
> > frame and code for doing the stack adjustments needs to be emitted in
> > the prologue and epilogue.
> 
> Right.  This is because you need to keep the stack aligned, so you can
> never have an empty frame.  Which brings us back to a variant of the
> previous poster's question: is the stack pointer supposed to be
> aligned, or not?

Yes, the stack pointer is supposed to aligned.  Your question
implies that the value specified in STARTING_FRAME_OFFSET affects
the stack alignment.  However, the frame sizes on the PA are
always rounded in compute_frame_size.  The STARTING_FRAME_OFFSET
is just one of a number of items that make up the final frame size.
STARTING_FRAME_OFFSET is not added into the total if the size 
returned by get_frame_size () is zero.  That's why we don't want
assign_stack_local_1 to artificially create a frame.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [PATCH]: Stack alignment

[John David Anglin]

> Right.  This is because you need to keep the stack aligned, so you can
> never have an empty frame.  Which brings us back to a variant of the
> previous poster's question: is the stack pointer supposed to be
> aligned, or not?

I have one additional point.  The alignment that reload does aligns
the stack to BIGGEST_ALIGNMENT.  This is an alignment suitable for
any local.  What the request doesn't do is align the stack to
STACK_BOUNDARY or PREFERRED_STACK_BOUNDARY.  If these are larger
than BIGGEST_ALIGNMENT, then the stack won't be properly aligned
unless the backend fixes the alignment.

I am questioning whether it is necessary to align the stack for locals
when there aren't any locals.  I am convinced that's not necessary on
the PA due to the way that we align the stack and what happens when
we have no frame.  If we have to make STARTING_FRAME_OFFSET a multiple
of BIGGEST_ALIGNMENT / 8 or whatever is needed to ensure that no
alignment occurs, then we have to waste space in every frame.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

[PATCH]: Stack alignment - take 2

[John David Anglin]

> Right.  This is because you need to keep the stack aligned, so you can
> never have an empty frame.  Which brings us back to a variant of the
> previous poster's question: is the stack pointer supposed to be
> aligned, or not?

Here is a second attempt to eliminate wasted space in the PA frame
due to unnecessary rounding and rounding to an alignment larger than
necessary.  I have introduced a macro STACK_ALIGNMENT_NEEDED which
allows a port to handle stack alignment itself.  The rounding of the
PA frame size is now done entirely in the backend in compute_frame_size.

This saves space in a number of ways.  First, it allows us to define
STARTING_FRAME_OFFSET to 8 in the 64-byte port and save 8 bytes in
every frame.  Secondly, the overall alignment in assign_stack_local_1
was being done to STACK_BOUNDARY.  This is set to a double word
boundary on the PA.  However, the register save block that follows
the locals in the PA frame only needs to be aligned to word boundary,
so we save a word about half the time by aligning to a word boundary.

I have also improved compute_frame_size.  We no longer allocate a
slot for %r3 when it is used as the frame pointer.  Finally, we don't
need to do a double word alignment before the floating point register
save block if we don't save any floating point registers.  The
alignment of the outgoing argument block is determined by the final
rounding to PREFERRED_STACK_BOUNDARY done at the end of compute_frame_size.

The preferred stack boundary on the 32-bit ports is 64 bytes.  If
unnecessary rounding just happens bump the frame size over the 64
byte boundary, then a lot of stack space could be wasted.

The patch has been tested on hppa2.0-hp-hpux11.11, hppa64-hp-hpux11.11,
and hppa-unknown-linux-gnu with no regressions.

In response to the question, the PA requires strict alignment of all
instructions and data.  Any misalignment of the stack pointer would
cause immediate problems in a non-trivial program.  So, the stack
pointer must be aligned.  However, we don't want assign_stack_local_1
to do it for us as it can't provide the alignment we need.

Ok for main?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2003-03-01  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* function.c (STACK_ALIGNMENT_NEEDED): New macro.  Default to 1.
	(assign_stack_local_1): Perform overall stack alignment only when
	STACK_ALIGNMENT_NEEDED is non-zero.
	* doc/tm.texi (STACK_ALIGNMENT_NEEDED): Document.

	* pa.c (compute_frame_size): Rename fsize to size.  Account for
	alignment to a word boundary before general register save block.  Only
	account for double-word alignment before floating point register save
	block if one or more are saved.  Don't allocate space for %r3 when
	frame pointer is needed.
	(hppa_expand_prologue): Include alignment to word boundary in local
	frame size.
	* pa.h (STARTING_FRAME_OFFSET): Define to 8 on both 32 and 64-bit ports.
	(STACK_ALIGNMENT_NEEDED): Define.

Index: function.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/function.c,v
retrieving revision 1.404
diff -u -3 -p -r1.404 function.c
--- function.c	28 Feb 2003 07:06:33 -0000	1.404
+++ function.c	28 Feb 2003 19:21:47 -0000
@@ -70,6 +70,10 @@ Software Foundation, 59 Temple Place - S
 #define LOCAL_ALIGNMENT(TYPE, ALIGNMENT) ALIGNMENT
 #endif
 
+#ifndef STACK_ALIGNMENT_NEEDED
+#define STACK_ALIGNMENT_NEEDED 1
+#endif
+
 /* Some systems use __main in a way incompatible with its use in gcc, in these
    cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN to
    give the same symbol without quotes for an alternative entry point.  You
@@ -566,16 +570,27 @@ assign_stack_local_1 (mode, size, align,
   frame_off = STARTING_FRAME_OFFSET % frame_alignment;
   frame_phase = frame_off ? frame_alignment - frame_off : 0;
 
-  /* Round frame offset to that alignment.
-     We must be careful here, since FRAME_OFFSET might be negative and
-     division with a negative dividend isn't as well defined as we might
-     like.  So we instead assume that ALIGNMENT is a power of two and
-     use logical operations which are unambiguous.  */
+  /* Round the frame offset to the specified alignment.  The default is
+     to always honor requests to align the stack but a port may choose to
+     do its own stack alignment by defining STACK_ALIGNMENT_NEEDED.  */
+  if (STACK_ALIGNMENT_NEEDED
+      || mode != BLKmode
+      || size != 0)
+    {
+      /*  We must be careful here, since FRAME_OFFSET might be negative and
+	  division with a negative dividend isn't as well defined as we might
+	  like.  So we instead assume that ALIGNMENT is a power of two and
+	  use logical operations which are unambiguous.  */
 #ifdef FRAME_GROWS_DOWNWARD
-  function->x_frame_offset = FLOOR_ROUND (function->x_frame_offset - frame_phase, alignment) + frame_phase;
+      function->x_frame_offset
+	= (FLOOR_ROUND (function->x_frame_offset - frame_phase, alignment)
+	   + frame_phase);
 #else
-  function->x_frame_offset = CEIL_ROUND (function->x_frame_offset - frame_phase, alignment) + frame_phase;
+      function->x_frame_offset
+	= (CEIL_ROUND (function->x_frame_offset - frame_phase, alignment)
+	   + frame_phase);
 #endif
+    }
 
   /* On a big-endian machine, if we are allocating more space than we will use,
      use the least significant bytes of those that are allocated.  */
Index: doc/tm.texi
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/doc/tm.texi,v
retrieving revision 1.204
diff -u -3 -p -r1.204 tm.texi
--- doc/tm.texi	25 Feb 2003 17:06:40 -0000	1.204
+++ doc/tm.texi	28 Feb 2003 19:21:48 -0000
@@ -2816,6 +2816,16 @@ value @code{STARTING_FRAME_OFFSET}.
 @c i'm not sure if the above is still correct.. had to change it to get
 @c rid of an overfull.  --mew 2feb93
 
+@findex STACK_ALIGNMENT_NEEDED
+@item STACK_ALIGNMENT_NEEDED
+Define to zero to disable final alignment of the stack during reload.
+The non-zero default for this macro is suitable for most ports.
+
+On ports where @code{STARTING_FRAME_OFFSET} is non-zero or where there
+is a register save block following the local block that doesn't require
+alignment to @code{STACK_BOUNDARY}, it may be beneficial to disable
+stack alignment and do it in the backend.
+
 @findex STACK_POINTER_OFFSET
 @item STACK_POINTER_OFFSET
 Offset from the stack pointer register to the first location at which
Index: config/pa/pa.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.c,v
retrieving revision 1.199
diff -u -3 -p -r1.199 pa.c
--- config/pa/pa.c	26 Feb 2003 16:29:22 -0000	1.199
+++ config/pa/pa.c	28 Feb 2003 19:21:48 -0000
@@ -3195,15 +3195,24 @@ compute_frame_size (size, fregs_live)
      int size;
      int *fregs_live;
 {
-  int i, fsize;
-
-  /* Space for frame pointer + filler. If any frame is allocated
-     we need to add this in because of STARTING_FRAME_OFFSET.
-
-     Similar code also appears in hppa_expand_prologue.  Change both
-     of them at the same time.  */
-  fsize = size + (size || frame_pointer_needed ? STARTING_FRAME_OFFSET : 0);
+  int freg_saved = 0;
+  int i, j;
 
+  /* The code in hppa_expand_prologue and hppa_expand_epilogue must
+     be consistent with the rounding and size calculation done here.
+     Change them at the same time.  */
+
+  /* We do our own stack alignment.  First, round the size of the
+     stack locals up to a word boundary.  */
+  size = (size + UNITS_PER_WORD - 1) & ~(UNITS_PER_WORD - 1);
+
+  /* Space for previous frame pointer + filler.  If any frame is
+     allocated, we need to add in the STARTING_FRAME_OFFSET.  We
+     waste some space here for the sake of HP compatibility.  The
+     first slot is only used when the frame pointer is needed.  */
+  if (size || frame_pointer_needed)
+    size += STARTING_FRAME_OFFSET;
+  
   /* If the current function calls __builtin_eh_return, then we need
      to allocate stack space for registers that will hold data for
      the exception handler.  */
@@ -3213,41 +3222,49 @@ compute_frame_size (size, fregs_live)
 
       for (i = 0; EH_RETURN_DATA_REGNO (i) != INVALID_REGNUM; ++i)
 	continue;
-      fsize += i * UNITS_PER_WORD;
+      size += i * UNITS_PER_WORD;
     }
 
   /* Account for space used by the callee general register saves.  */
-  for (i = 18; i >= 3; i--)
+  for (i = 18, j = frame_pointer_needed ? 4 : 3; i >= j; i--)
     if (regs_ever_live[i])
-      fsize += UNITS_PER_WORD;
-
-  /* Round the stack.  */
-  fsize = (fsize + 7) & ~7;
+      size += UNITS_PER_WORD;
 
   /* Account for space used by the callee floating point register saves.  */
   for (i = FP_SAVED_REG_LAST; i >= FP_SAVED_REG_FIRST; i -= FP_REG_STEP)
     if (regs_ever_live[i]
-	|| (! TARGET_64BIT && regs_ever_live[i + 1]))
+	|| (!TARGET_64BIT && regs_ever_live[i + 1]))
       {
-	if (fregs_live)
-	  *fregs_live = 1;
+	freg_saved = 1;
 
 	/* We always save both halves of the FP register, so always
 	   increment the frame size by 8 bytes.  */
-	fsize += 8;
+	size += 8;
       }
 
+  /* If any of the floating registers are saved, account for the
+     alignment needed for the floating point register save block.  */
+  if (freg_saved)
+    {
+      size = (size + 7) & ~7;
+      if (fregs_live)
+	*fregs_live = 1;
+    }
+
   /* The various ABIs include space for the outgoing parameters in the
-     size of the current function's stack frame.  */
-  fsize += current_function_outgoing_args_size;
+     size of the current function's stack frame.  We don't need to align
+     for the outgoing arguments as their alignment is set by the final
+     rounding for the frame as a whole.  */
+  size += current_function_outgoing_args_size;
 
   /* Allocate space for the fixed frame marker.  This space must be
      allocated for any function that makes calls or otherwise allocates
      stack space.  */
-  if (!current_function_is_leaf || fsize)
-    fsize += TARGET_64BIT ? 16 : 32;
+  if (!current_function_is_leaf || size)
+    size += TARGET_64BIT ? 16 : 32;
 
-  return ((fsize + PREFERRED_STACK_BOUNDARY / 8 - 1)
+  /* Finally, round to the preferred stack boundary.  */
+  return ((size + PREFERRED_STACK_BOUNDARY / 8 - 1)
 	  & ~(PREFERRED_STACK_BOUNDARY / 8 - 1));
 }
 
@@ -3313,8 +3330,8 @@ pa_output_function_prologue (file, size)
 void
 hppa_expand_prologue ()
 {
-  int size = get_frame_size ();
   int merge_sp_adjust_with_store = 0;
+  int size = get_frame_size ();
   int i, offset;
   rtx insn, tmpreg;
 
@@ -3322,13 +3339,12 @@ hppa_expand_prologue ()
   fr_saved = 0;
   save_fregs = 0;
 
-  /* Allocate space for frame pointer + filler. If any frame is allocated
-     we need to add this in because of STARTING_FRAME_OFFSET.
-
-     Similar code also appears in compute_frame_size.  Change both
-     of them at the same time.  */
-  local_fsize = size + (size || frame_pointer_needed
-			? STARTING_FRAME_OFFSET : 0);
+  /* Compute total size for frame pointer, filler, locals and rounding to
+     the next word boundary.  Similar code appears in compute_frame_size
+     and must be changed in tandem with this code.  */
+  local_fsize = (size + UNITS_PER_WORD - 1) & ~(UNITS_PER_WORD - 1);
+  if (local_fsize || frame_pointer_needed)
+    local_fsize += STARTING_FRAME_OFFSET;
 
   actual_fsize = compute_frame_size (size, &save_fregs);
 
Index: config/pa/pa.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.h,v
retrieving revision 1.184
diff -u -3 -p -r1.184 pa.h
--- config/pa/pa.h	26 Feb 2003 16:29:22 -0000	1.184
+++ config/pa/pa.h	28 Feb 2003 19:21:48 -0000
@@ -676,10 +676,17 @@ extern struct rtx_def *hppa_pic_save_rtx
 /* Offset within stack frame to start allocating local variables at.
    If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
    first local allocated.  Otherwise, it is the offset to the BEGINNING
-   of the first local allocated.  The start of the locals must lie on
-   a STACK_BOUNDARY or else the frame size of leaf functions will not
-   be zero.  */
-#define STARTING_FRAME_OFFSET (TARGET_64BIT ? 16 : 8)
+   of the first local allocated.
+
+   On the 32-bit ports, we reserve one slot for the previous frame
+   pointer and one fill slot.  The fill slot is for compatibility
+   with HP compiled programs.  On the 64-bit ports, we reserve one
+   slot for the previous frame pointer.  */
+#define STARTING_FRAME_OFFSET 8
+
+/* Define STACK_ALIGNMENT_NEEDED to zero to disable final alignment
+   of the stack.  The default is to align it to STACK_BOUNDARY.  */
+#define STACK_ALIGNMENT_NEEDED 0
 
 /* If we generate an insn to push BYTES bytes,
    this says how many the stack pointer really advances by.

Re: [PATCH]: Stack alignment - take 2

[Richard Henderson]

On Sat, Mar 01, 2003 at 02:31:50PM -0500, John David Anglin wrote:
> 	* function.c (STACK_ALIGNMENT_NEEDED): New macro.  Default to 1.
> 	(assign_stack_local_1): Perform overall stack alignment only when
> 	STACK_ALIGNMENT_NEEDED is non-zero.
> 	* doc/tm.texi (STACK_ALIGNMENT_NEEDED): Document.
> 
> 	* pa.c (compute_frame_size): Rename fsize to size.  Account for
> 	alignment to a word boundary before general register save block.  Only
> 	account for double-word alignment before floating point register save
> 	block if one or more are saved.  Don't allocate space for %r3 when
> 	frame pointer is needed.
> 	(hppa_expand_prologue): Include alignment to word boundary in local
> 	frame size.
> 	* pa.h (STARTING_FRAME_OFFSET): Define to 8 on both 32 and 64-bit ports.
> 	(STACK_ALIGNMENT_NEEDED): Define.

Ok.

Seems like this ought to be avoidable by intelligently rearranging
the way things are layed out in the stack frame, but that may not
be an option based on ABI constrints.


r~