[PATCH] Treat a sibling call as though it does a wild read

[John David Anglin]

On 23-Dec-14, at 7:28 PM, H.J. Lu wrote:

> On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin <dave.anglin@bell.net 
> > wrote:
>> On 23-Dec-14, at 5:37 PM, H.J. Lu wrote:
>>
>>> In this case,  arguments are passed in registers.  Isn't the  
>>> optimization
>>> disabled for ia32, which passes arguments on stack, even before your
>>> change?
>>
>>
>> It's not disabled in dse.c.  Possibly, this occurs for some cases  
>> for ia32
>> in ix86_function_ok_for_sibcall.
>>
>> The problem in dse is in how to distinguish stores used for  
>> arguments from
>> those used for general
>> manipulation of data.  It seems the argument data for the call to  
>> bar in the
>> testcase are copied through
>> frame memory on x86_64.
>>
>> We have two situations:
>>
>>  /* This field is only used for the processing of const functions.
>>     These functions cannot read memory, but they can read the stack
>>     because that is where they may get their parms.  We need to be
>>     this conservative because, like the store motion pass, we don't
>>     consider CALL_INSN_FUNCTION_USAGE when processing call insns.
>>     Moreover, we need to distinguish two cases:
>>     1. Before reload (register elimination), the stores related to
>>        outgoing arguments are stack pointer based and thus deemed
>>        of non-constant base in this pass.  This requires special
>>        handling but also means that the frame pointer based stores
>>        need not be killed upon encountering a const function call.
>>     2. After reload, the stores related to outgoing arguments can be
>>        either stack pointer or hard frame pointer based.  This means
>>        that we have no other choice than also killing all the frame
>>        pointer based stores upon encountering a const function call.
>>     This field is set after reload for const function calls.  Having
>>     this set is less severe than a wild read, it just means that all
>>     the frame related stores are killed rather than all the  
>> stores.  */
>>  bool frame_read;
>>
>> Case 1 is incorrect for sibling calls as the call arguments are  
>> frame or
>> argument pointer based
>> when they are not passed in registers.
>>
>> When we don't have a const function, dse assumes:
>>
>>        /* Every other call, including pure functions, may read any  
>> memory
>>           that is not relative to the frame.  */
>>        add_non_frame_wild_read (bb_info);
>>
>> Again, this is incorrect for sibling calls (i.e., dse in general  
>> assumes
>> that call arguments are register
>> or stack pointer based before reload).
>>
>
> This optimization is done on purpose to fix:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194
>
> Unless it is also incorrect for x86, why not only disable it for
> your target without penalizing x86 through a target hook.


I tend to think the old code was wrong for x86 but I recognize the  
lost optimization.

This isn't an optimization issue on hppa.  It's a wrong code bug.

Richard wrote regarding 44194:
"So we create a stack representation to copy it to the stack temporary  
(which both
are unneeded). We should see that we can avoid the temporary at all as  
there is no
aggregate use of the struct left. At least we should avoid the 2nd  
temporary."
The temporaries are still there and they are the problem.
I don't see that this is a target issue.  There were nothing more than  
argument stores in
the testcase for the PR 55023 and they were deleted by dse.

Dave
--
John David Anglin	dave.anglin@bell.net