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

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

Ian Lance Taylor wrote:
> Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>
>   
>>> There is special code in calls.c to mark the stack usage of const
>>> functions: around line 2700, in expand_call.  It seems to me that if
>>> store_one_arg comes up with a different address, because it calls
>>> memory_address, you should put that address also in
>>> CALL_INSN_FUNCTION_USAGE.  That ought to fix the immediate problem as
>>> well.
>>>       
>> Does dse.c take into account CALL_INSN_FUNCTION_USAGE at all?  Flow.c did.
>>     
>
> It does.  The problem is that CALL_INSN_FUNCTION_USAGE has a mem with
> virtual-outgoing-args, but the actual store is done via a reg.  The
> address was copied into the reg due to -fforce-addr.  This can be seen
> in the RTL which Uros posted.
>
> dse.c is not smart enough to see that the address in the
> CALL_INSN_FUNCTION_USAGE is the same as the address in the register.
> This is an issue that only arises for const functions.
>
> Ian
>   
There are two places where dse uses knowledge of an address being
frame_related:
1) in this code for const functions.
2) in code that deletes stores that die because the function ends.

(1) is sensitive to this problem in that it will get the wrong answer
for an arg to a const function.  (2) will only miss opportunities to
remove dead insns if some frame based stores are not identified. 

it is not that dse is not smart enough to see this. dse is cselib based
which is a local analysis, and it is not smart enough to see this.  I
believe that the reason that flow never ran into this problem was that
flow did not try to do this.  My dse is more aggressive than flow was.

As to ian's point in the prior mail:

> Some processors don't have register offset addressing, so all access
> to the stack is done by copying and adding registers.  It seems to me
> that such processors might have trouble with dse.c as currently
> written.


I am surprised that this has not bitten us before.  this code has been
in since the dataflow merge and if some arch really had to do this, i
would have expected we would have seen this before.  I can understand
being lucky about this because -fforce-addr is a rarely used flag, but i
have trouble seeing how this could even bootstrap on a processor that
had to do it.

I can easily turn off the optimization if the -fforce-addr is given.  I
guess we could add a flag the the md files of the processors that need
to do this of the form: PROCESSOR_LOADS_STACK_ADDRESSES_INTO_REGS or
something like this.
I am a little scared about having to thread a new flag onto registers
thru the optimization stack at this point. 

Kenny

Kenny