On Tue, May 24, 2011 at 01:42:11PM +0200, Eric Botcazou wrote:
> > That is so that the argp (or framep) will be properly handled during
> > CSELIB, as constant VALUE that is never changed through the function. And
> > additionally to find out if we can do the replacement (we can't if argp
> > resp. framep isn't eliminated and could therefore occur in the real code).
>
> OK, thanks for the explanation.
So, would you like me to redo my patch on top of your patch, test it and
submit?
> > This comment is not 100% accurate. The reason we do that is to use the
> > much more compact DW_OP_fbreg* if possible compared to huge location lists,
> > especially for !frame_pointer_needed where the location list would need to
> > have another entry whenever sp changes. dwarf2out.c rewrites argp/framep
> > if eliminated into DW_OP_fbreg (with some offset when needed).
>
> Adjusted.
Ok.
> > While I had part of this in my patch too, it is unnecessary, if prologue_bb
> > is set to non-NULL only when we want to use it, then we know fp_cfa_offset
> > is not -1. By computing prologue_bb always, it will call fp_setter
> > unnecessarily for every insn in the prologue bb for !frame_pointer_needed
> > or for DRAP. I guess adding a comment instead of this would be better.
>
> The fp_cfa_offset != -1 was before the call to fp_setter in my first patch.
> :-)
If you think it makes the code clearer, the extra comparison for at most
each stmt in the prologue_bb is probably noise compile time wise.
>
> Revised version attached.
Fine with me, but I'm not a reviewer of this part of GCC...
Jakub
