[PATCH 1/7]: Ping2: Merge from Stack Branch - New data in function.h

Ye, Joey joey.ye@intel.com
Mon May 26 15:20:00 GMT 2008


Richard, Thanks for your review. See my comments below:

Thanks - Joey

> Index: function.h
> I have difficulties in deriving differences between all the
stack_alignment
> values and predicates below.  Their names aren't too descriptive IMHO.

> For example stack_alignment_used could be better named
> max_used_stack_slot_alignment (or "ss" for stack slot, if that will
> occur more often).

> Is stack_alignment_estimated the estimated alignment of the stack
pointer as
> set up by the caller?  If so please say that in the comment.
See more descriptive comments in attached patch.


>> +  /* Nonzero if need_frame_pointer has been set.  */
>> +  bool need_frame_pointer_set;

>The description sounds as if this is a redundant flag.  I suppose you
mean
>"Nonzero if a frame-pointer was set up"?  In this case adjust the
comment
>and rename the flag to simply frame_pointer_set.
It means flag need_frame_pointer was set.

>> +  /* Nonzero if, by estimation, current function stack needs
>> realignment. */
>> +  bool stack_realign_needed;
> stack_realign_really_needed.  But why not simply re-use the above
flag?
> It sounds like the meaning just "switches" after reload?
We estimate required stack alignment before reload. The stack realign
conclusion before reload is stored in flag stack_realign_needed. After
reload the alignment may change and will be finalized. The conclusion
after reload is stored in flag stack_realign_really. Yes we can reuse
the same flag for both pre and post reload. In the attached patch
stack_realign_really is removed.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: function.h-0526.patch
Type: application/octet-stream
Size: 3851 bytes
Desc: function.h-0526.patch
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20080526/b45edbdd/attachment.obj>


More information about the Gcc-patches mailing list