Re: [patch,rfc] PR25196

[Ian Lance Taylor <ian at airs dot com>]

Steven Bosscher <stevenb@suse.de> writes:

> According to a comment in postreload.c:move2add_note_store(), we can
> have pushes without REG_INC notes:
>   /* Some targets do argument pushes without adding REG_INC notes.  */
> 
> So we need to go look for those {pre,post}{decrements,increments} ourselves. 
> With this patch, we just do what postreload.c does.
> 
> I don't understand why it would be OK to not have REG_INC notes here,
> so anyone who can explain this, please help :-)

My understanding is that historically the compiler does not add
REG_INC notes for the stack pointer itself.  On some machines the
stack pointer is routinely pushed and popped when passing function
arguments.  Adding REG_INC notes for that case isn't particularly
helpful.  In particular, note that emit_single_push_insn does not add
a REG_INC note.  In general the compiler only adds a REG_INC note when
it discovers a case where it can use auto-increment/decrement; it
doesn't add a REG_INC note to an insn which was initially generated to
use auto-increment/decrement.

> Index: postreload-gcse.c
> ===================================================================
> --- postreload-gcse.c   (revision 108853)
> +++ postreload-gcse.c   (working copy)
> @@ -676,6 +676,17 @@ record_last_set_info (rtx dest, rtx sett
>            /* Ignore pushes, they clobber nothing.  */
>            && ! push_operand (dest, GET_MODE (dest)))
>      record_last_mem_set_info (last_set_insn);
> +
> +  /* ??? Some targets do argument pushes without adding REG_INC notes.
> +     See e.g. PR25196, where a pushsi2 on i386 doesn't have REG_INC
> +     notes.  */
> +  if (MEM_P (dest))
> +    {
> +      dest = XEXP (dest, 0);
> +      if (GET_CODE (dest) == PRE_INC || GET_CODE (dest) == POST_INC
> +         || GET_CODE (dest) == PRE_DEC || GET_CODE (dest) == POST_DEC)
> +       record_last_reg_set_info (last_set_insn, REGNO (XEXP (dest, 0)));
> +    }
>  }

It should work to do this:

#ifdef PUSH_ROUNDING
  if (MEM_P (dest))
    {
      rtx addr = XEXP (dest, 0);
    
      if (GET_RTX_CLASS (GET_CODE (addr)) == RTX_AUTOINC)
        record_last_reg_set_info (last_set_insn, REGNO (XEXP (addr, 0)));
    }
#endif

We should only see a push to the stack pointer without a REG_INC note
if PUSH_ROUNDING is defined.  Compare to the similar code in cse_insn
in cse.c.  Alternatively, see cselib_invalidate_rtx in cselib.c.

Anyhow, let's not worry too much about PUSH_ROUNDING, which is just
gilding the lily.  I'll approve your patch if you use GET_RTX_CLASS,
and if it passes bootstrap and testing.  Please also add the test
case.

Thanks.

Ian