Patch ping

Jakub Jelinek jakub@redhat.com
Mon Nov 19 07:53:00 GMT 2012


On Sat, Nov 17, 2012 at 11:10:04AM -0800, Richard Henderson wrote:
> On 11/16/2012 01:10 AM, Jakub Jelinek wrote:
> > Hi!
> > 
> > - PR54921 invalidate sp in cselib on fp setter insn
> >   http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02035.html
> >   (perhaps in light of PR54402 the single_succ (ENTRY_BLOCK_PTR)
> >   from the patch should be nuked)
> 
> > +      rtx expr = find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX);
> > +      if (expr)
> > +	pat = XEXP (expr, 0);
> > +      if (GET_CODE (pat) == SET
> > +	  && SET_DEST (pat) == hard_frame_pointer_rtx)
> > +	cselib_invalidate_rtx (stack_pointer_rtx);
> > +      else if (GET_CODE (pat) == PARALLEL)
> 
> This is only one possible way that FP can be set.
> The others are with CFA_DEF_CFA or CFA_ADJUST_CFA.
> 
> Given 
> 
> +      && frame_pointer_needed
> +      && RTX_FRAME_RELATED_P (insn)
> 
> is there any reason to do more than
> 
>        && modified_in_p (hard_frame_pointer_rtx, insn)
> 
> ?

I'd prefer to only invalidate the stack pointer on the first assignment
to the hard pointer.  If the cselib link between sp and hfp is already
broken, invalidating sp will only result in worse code.  Dunno if there
are any targets that adjust the hard frame pointer after it has been set
once or similar.
Perhaps we could walk here CSELIB_VAL_PTR (hfpval)->locs here, and look
if any rtls in there have find_base_term (x->loc) == find_base_term
(stack_pointer_rtx), and only if yes, invalidate (and guard it by the
modified_in_p test).
BTW, var-tracking.c uses a similar test.

	Jakub



More information about the Gcc-patches mailing list