Re: Stack Reorganization Patch

["Naveen Sharma, Noida" <naveens at noida dot hcltech dot com>]

Hi,

> > This is a re-post of my previous patch at 
> >   http://gcc.gnu.org/ml/gcc-patches/2003-01/msg00019.html
> >
> > I would request the review of the patch and let me know 
> > the issues in its acceptance.
> 
> I do not have authority to approve this patch myself but I can make
> some comments.

Thanks for looking at it. Your comments would help me improve patch further.

Here are replies to some of your comments; 
 
> (1) I do not see why you have introduced a mechanism for targets to
>     disable this optimization.  It shouldn't have machine-specific
>     reasons why it won't work or isn't an improvement.  This merely
>     adds complexity and makes it harder to test.
 
It is cautiousness on my part. I did just for the sake of testing 
separately and enable for only those targets which benefit from it.
(although it is very much a generic optimization). We can remove 
this complication.
 
> (2) You need testing on more architectures.  If you turn on the
>     optimization unconditionally, you can test against most
>     interesting architectures by using cross compilers to simulator
>     targets.  There are detailed instructions for this on the GCC
>     website.  Pick five popular architectures, verify that there are
>     no regressions, and do a code size comparison for each one.
>     (Speed measurements are unlikely to be meaningful for a
>     simulator.)

I can test it using simulators but it is slightly
difficult for me because of resources required.
I will need help from people who have expertise in 
those architectures.

Please suggest architectures that might benefit 
from this optimization (Possibly HPPA, ARM-THUMB, and 
possibly alpha). 

 
>     /stack-reorg.c/0/dummy timestamp//
> 
>     at the end.  Note that as a matter of convention we prefer to use
>     dashes in file names, not underscores.

I see, I will rename the file.

> (5) Must you call stack_reorg() from the middle of reload?  Can't you
>     do it immediately before or after?

stack_reorg () will have little effect before reload because most of 
the stack allocations are from within reload. And replacing stack 
pseudos with their normal form after reload () turns out to be 
complicated (validation of changed rtx's becomes part of
of stack-reorg, a task that reload is already doing). 
So calling stack_reorg () from within reload turns out out
be 1) simpler and 2) reload's code for generating valid 
rtl can be used. 

Some discussion on this issue is at:
  http://gcc.gnu.org/ml/gcc/2002-05/threads.html#02838

> (6) You've got code in stack_reorg() to fix up DECL_RTL.  This
>     *should* be unnecessary -- nothing should be looking at DECL nodes
>     this late in rest_of_compilation.  However, I could be unaware of
>     some horrible interaction issue.  Please take that out and see if
>     anything breaks.

Fixing DECL_RTLs is done for generation of debugging information.
(final.c uses DECL_RTLs)
 
I will reply to rest of the comments shortly.

Thanks a lot !!

Best Regards,
  Naveen Sharma.