On Wed, 2020-04-08 at 00:55 +0200, Jakub Jelinek wrote:
> Hi!
>
> The following testcase shows two separate issues caused by the cselib
> changes.
> One is that through the cselib sp tracking improvements on
> ... r12 = rsp; rsp -= 8; push cst1; push cst2; push cst3; call
> rsp += 32; rsp -= 8; push cst4; push cst5; push cst6; call
> rsp += 32; rsp -= 8; push cst7; push cst8; push cst9; call
> rsp += 32
> reload_cse_simplify_set decides to optimize the rsp += 32 insns
> into rsp = r12 because cselib figures that the r12 register holds the right
> value. From the pure cost perspective that seems like a win and on its own
> at least for -Os that would be beneficial, except that there are those
> rsp -= 8 stack adjustments after it, where rsp += 32; rsp -= 8; is optimized
> into rsp += 24; by the csa pass, but rsp = r12; rsp -= 8 can't. Dunno
> what to do about this part, the PR has a hack in a comment.
Yea, what to do here is a bit unclear. I'd guess the CSA improvements happen
more often that cselib finding a register that just happens to have the right
value lying around. But that's speculation with no hard data on my part...
>
> Anyway, the following patch fixes the other part, which isn't a missed
> optimization, but a wrong-code issue. The problem is that the pushes of
> constant are on x86 represented through PRE_MODIFY and while
> move2add_note_store has some code to handle {PRE,POST}_{INC,DEC} without
> REG_INC note, it doesn't handle {PRE,POST}_MODIFY (that would be enough
> to fix this testcase). But additionally it looks misplaced, because
> move2add_note_store is only called on the rtxes that are stored into,
> while RTX_AUTOINC can happen not just in those, but anywhere else in the
> instruction (e.g. pop insn can have autoinc in the SET_SRC MEM).
> REG_INC note seems to be required for any autoinc except for stack pointer
> autoinc which doesn't have those notes, so this patch just handles
> the sp autoinc after the REG_INC note handling loop.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-04-07 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/94516
> * postreload.c: Include rtl-iter.h.
> (reload_cse_move2add): Handle SP autoinc here by FOR_EACH_SUBRTX_VAR
> looking for all MEMs with RTX_AUTOINC operand.
> (move2add_note_store): Remove {PRE,POST}_{INC,DEC} handling.
>
> * gcc.dg/torture/pr94516.c: New test.
OK
jeff
