Re: PREFERRED_STACK_BOUNDARY code tweaks [+ tree question]

[Jan Hubicka <hubicka at atrey dot karlin dot mff dot cuni dot cz>]

> > This patch adds code to keep track of preferred stack boundary for each
> > function.
> 
> I like this a lot, in principal.
Thanks..
I expected this patch to be ignored or refused, since there are so many
problematic parts inside...
> 
> > The patch also attempts to fix problem in reload with stack_alignment_needed
> > optimization.  It forces frame alignment always to BIGGEST_ALIGNMENT, that
> > causes stack_align_needed to be always at least BIGGEST_ALIGNMENT. Because
> > BIGGEST_ALIGNMENT == PREFERRED_STACK_BOUNDARY now, the optimization is not
> > effective anymore.
> 
> Well, on x86 that's the case.  Are you sure that's true every where
> else as well?  I havn't checked myself, but it surely seems likely
> to be wrong somewhere.

I think that only machine, where PREFERRED_STACK_BOUDNARY is defined
is i386, and only on such machines stack_align_needed is usefull.
I've added this bit myself and it is unused so far.

> Well, on a previous iteration of the loop we could have allocated
> new stack slots for new spills, changing the size of the stack frame,
> making it not a multiple of our required alignment.

It is not clear to me, why we need frame to be multiple of the biggest
alignment.

The allocation code in function.c most probably don't need this, so is this
done only in order to make prologue/epilogue code happy (because it expect
frame_size to be multiple of BIGGEST_ALIGNMENT?)
If so, I think correct solution is to remove the alignment code here,
and modify all prologue/epilogue expanders to align the frame size.
I think I can do it.

Or is this necesary for machines where frame grows in reversed order than
stack? Are there any?

Or is this required somehow for the elimination code?

> > Last change I've implemented is simple optimization of
> > PREFERRED_STACK_BOUNDARY. The calculated stack boundaries are now stored to
> > DECL_ALIGN of the function and used by call code to avoid unnecesary
> > alignments. This optimization seems to save slighly more than 1% of code size
> > in XaoS.  I am quite nervous about overusing DECL_ALIGN for such purpose.
> 
> Mark or Jason, could you comment on the advisability of this?
> It _seems_ like it'd be ok, but...

In longer term, we can perhaps develop some was to save such information
about known function (inlining code, list of clobbered register and some
other info as well), so perhaps we can create something like
struct perm_function_data and point to it from the field currently used
for rtl inlining.
> 
> > !       preferred_stack_boundary /= BITS_PER_UNIT;
> > !       if (preferred_stack_boundary != BITS_PER_UNIT)
> > ! 	args_size->var = round_up (args_size->var, preferred_stack_boundary);
> 
> This can't be right.  What units is preferred_stack_boundary now in?
> Perhaps you meant `(preferred_stack_boundary > 1)' ?

yes... stupid thinko..
> 
> > + #ifdef PREFERRED_STACK_BOUNDARY
> > +   int preferred_stack_boundary = PREFERRED_STACK_BOUNDARY;
> > + #else
> > +   int preferred_stack_boundary = 0;
> > + #endif
> 
> I believe PREFERRED_STACK_BOUNDARY is supposed to default to 
> STACK_BOUNDARY.  Does this even work setting it to 0?

It doesn't matter here, since preferred_stack_boundary is used only
to increase cfun->preferred_stack_boundary, that is high enought
(STACK_BOUNDARY) when PREFERRED_STACK_BOUNDARY is not defined.

The code have no effect and I use the variable only to keep #ifdef
noise down (I need to pass it to some functions too).
Perhaps I can default it to the STACK_BOUNDARY to make this less confusing.
> 
> > *************** emit_library_call VPARAMS((rtx orgfun, i
> > *** 2671,2676 ****
> > --- 2695,2705 ----
> >   
> >     push_temp_slots ();
> >   
> > +   /* Ensure current function's preferred stack boundary is at least
> > +      what we need.  */
> > +   if (cfun->preferred_stack_boundary < PREFERRED_STACK_BOUNDARY)
> > +     cfun->preferred_stack_boundary = PREFERRED_STACK_BOUNDARY;
> 
> It'd be nice to not have to push this up so far, though I realize
> you don't currently have the info to do otherwise.  Nothing in
> libgcc requires more than 8 byte alignment.

I plan to improve that in followup patch. We would probably need to introduce
new target macro, or use some kind of heruistic (such as using the largest
alignment needed for the parameters/return value, wich is generally the
largest alignment needed by the function in libgcc2, but this is quite
kludge).

I would like to not add new problematic changes to this patch...
> 
> > + 	fprintf (file, " preffered_stack_boundary %d", DECL_ALIGN (node));
> 
> Typo.
One of my favorite spelling errors...

Thanks
Honza