[PATCH, rtl-optimization]: Fix PR rtl-optimization/33638

[Ian Lance Taylor]

"Uros Bizjak" <ubizjak@gmail.com> writes:

> An (less intrusive?) variant of my previous patch is to change call to
> memory_address in emit_push_insn() to memory_address_noforce(). I
> can't see the benefit of
> 
> load reg <- (SP + off)
> store (reg) <- regX
> 
> since IMO the address of the pushed argument won't be CSE'd with any
> other address in any way.

It can be CSE'd if you make two function calls in a row.

Whether that CSE is useful is admittedly another question.

> BTW: I have tried to update arg->stack just after the call to
> emit_push_insn in store_one_arg with SET_DEST (PATTERN (get_last_insn
> ())) to update CALL_INSN_FUNCTION_USAGE, but IMO, this is too hacky.

But CALL_INSN_FUNCTION_USAGE has not been set at that point.  The code
calls store_one_arg, then adds an entry to CALL_INSN_FUNCTION_USAGE
for a const function.  You would just have to arrange for the call to
store_one_arg to return the actual address that it uses.

This is not a complete solution, as nothing prevents that address from
getting munged in some other way between expand and dse.

Arguably the bug is assuming that a note does a good job of conveying
that an address is being used.  This only works if the address doesn't
change in any way, or if the other optimizations are smart enough to
pick up (which in general they aren't).  The note was added for PR
12372, which was about a similar case for a sibcall: the store was
getting deleted before the jump.  It wouldn't be surprising if the
test case in PR 12372 failed with -fforce-addr when using flow.

Ian