This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Treat a sibling call as though it does a wild read
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: John David Anglin <dave dot anglin at bell dot net>
- Cc: Jeff Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 23 Dec 2014 18:47:51 -0800
- Subject: Re: [PATCH] Treat a sibling call as though it does a wild read
- Authentication-results: sourceware.org; auth=none
- References: <BLU437-SMTP45F1BE4C7DB75367213AE3977F0 at phx dot gbl> <547C9DEC dot 6030803 at redhat dot com> <BLU436-SMTP64BBC661D50C8981BBE1A397780 at phx dot gbl> <5486039B dot 1000107 at redhat dot com> <BLU436-SMTP83AF75FE3865ACE5738A6797640 at phx dot gbl> <548627E0 dot 2020206 at redhat dot com> <BLU437-SMTP10183F1B6B8CC2A828A43A7976D0 at phx dot gbl> <CAMe9rOoE2-AC_J8S8=pyXP4A0R7mfm_1wfK-u4dL_AVFWKCkvA at mail dot gmail dot com> <BLU436-SMTP75AD0060523548145EE9AC97570 at phx dot gbl> <CAMe9rOq5JX-k6aHO7LY4b_sAYK8B9uanKe3CvKKrA7Yd0GVz5A at mail dot gmail dot com> <BLU437-SMTP86350CEEF1E4CBC298B9F97570 at phx dot gbl> <CAMe9rOp==6XQfdC7Hdz9Acr_xLLc8dh7L-EXXkM-sv-BxL3mnQ at mail dot gmail dot com> <BLU436-SMTP12FEF052F357DB1BA18F4197540 at phx dot gbl>
On Tue, Dec 23, 2014 at 6:16 PM, John David Anglin <dave.anglin@bell.net> wrote:
> 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.
I run gcc.dg/pr55023.c on x86-64 and x86 without your fix. It works
fine. Can you find a testcase, failing on x86-64 and x86, which is
fixed by your change? If not, this bug doesn't affect x86 and you
should find another way to fix your target problem without introducing
a regression on x86.
> 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.
>
Thanks.
--
H.J.