This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Pass manager: add support for termination of pass list
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Martin LiÅka <mliska at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 4 Nov 2015 15:46:06 +0100
- Subject: Re: [PATCH] Pass manager: add support for termination of pass list
- Authentication-results: sourceware.org; auth=none
- References: <56263B07 dot 1010900 at suse dot cz> <562758A9 dot 3070309 at suse dot cz> <CAFiYyc0OWMO9ycyfds2rWOmi0j4e9pAhzEwtbKUujnK_xEwOrg at mail dot gmail dot com> <562775F3 dot 1050609 at suse dot cz> <CAFiYyc3gHUH=n7oep2Ynjdqks16rm_LmPXQPf5PNpFB5Go=RCQ at mail dot gmail dot com> <5628C262 dot 8000205 at suse dot cz> <CAFiYyc3GwQu58mmdEdWLGB3+iODREUC-KHBXhAWbz3+_4cJLNQ at mail dot gmail dot com> <562F6FC6 dot 1020200 at suse dot cz> <CAFiYyc2Qh8CjmKCwK8S=K4S33C6gR6AYf86jepzgMLPmZ5r3yA at mail dot gmail dot com> <562F9889 dot 2070704 at suse dot cz> <CAFiYyc1MZsOhgy+s9rmetE5T6w2jLdF0uR6ieuD5eBBjFaDoXg at mail dot gmail dot com> <5631EBA9 dot 2040105 at suse dot cz> <CAFiYyc1=T67BqzaKrE8=g=uf29beTY=xQUQNBBW_P0JAOov=aA at mail dot gmail dot com> <56323219 dot 4060802 at suse dot cz> <CAFiYyc304hRCTumKK=WSQHrYiyr8dAH7+MFUW7Fd-NR-GvpG4w at mail dot gmail dot com> <56335BB5 dot 8090408 at suse dot cz> <CAFiYyc18qFKdvePFHrEJT75pU_T+somDxdwOtp514Dc17OqfBA at mail dot gmail dot com> <56336858 dot 4060905 at suse dot cz> <CAFiYyc1gZoy4ju-jpkLzBaEGcZH0_z559jNnVVz2WGKWciV-YQ at mail dot gmail dot com> <5638C127 dot 5060508 at suse dot cz> <CAFiYyc0HBd0SiuvWhJg7nOJhRY++zh0fHGe=Z06nS1S0wmD=Fg at mail dot gmail dot com> <5639E03B dot 5000503 at suse dot cz>
On Wed, Nov 4, 2015 at 11:38 AM, Martin LiÅka <mliska@suse.cz> wrote:
> On 11/03/2015 03:44 PM, Richard Biener wrote:
>> On Tue, Nov 3, 2015 at 3:13 PM, Martin LiÅka <mliska@suse.cz> wrote:
>>> On 11/03/2015 02:46 PM, Richard Biener wrote:
>>>> On Fri, Oct 30, 2015 at 1:53 PM, Martin LiÅka <mliska@suse.cz> wrote:
>>>>> On 10/30/2015 01:13 PM, Richard Biener wrote:
>>>>>> So I suggest to do the push/pop of cfun there.
>>>>>> do_per_function_toporder can be made static btw.
>>>>>>
>>>>>> Richard.
>>>>>
>>>>> Right, I've done that and it works (bootstrap has been currently running),
>>>>> feasible for HSA branch too.
>>>>>
>>>>> tree-pass.h:
>>>>>
>>>>> /* Declare for plugins. */
>>>>> extern void do_per_function_toporder (void (*) (function *, void *), void *);
>>>>>
>>>>> Attaching the patch that I'm going to test.
>>>>
>>>> Err.
>>>>
>>>> + cgraph_node::get (current_function_decl)->release_body ();
>>>> +
>>>> + current_function_decl = NULL;
>>>> + set_cfun (NULL);
>>>>
>>>> I'd have expected
>>>>
>>>> tree fn = cfun->decl;
>>>> pop_cfun ();
>>>> gcc_assert (!cfun);
>>>> cgraph_node::get (fn)->release_body ();
>>>>
>>>> here.
>>>
>>> Yeah, that works, but we have to add following hunk:
>>>
>>> diff --git a/gcc/function.c b/gcc/function.c
>>> index aaf49a4..4718fe1 100644
>>> --- a/gcc/function.c
>>> +++ b/gcc/function.c
>>> @@ -4756,6 +4756,13 @@ push_cfun (struct function *new_cfun)
>>> void
>>> pop_cfun (void)
>>> {
>>> + if (cfun_stack.is_empty ())
>>> + {
>>> + set_cfun (NULL);
>>> + current_function_decl = NULL_TREE;
>>> + return;
>>> + }
>>> +
>>> struct function *new_cfun = cfun_stack.pop ();
>>> /* When in_dummy_function, we do have a cfun but current_function_decl is
>>> NULL. We also allow pushing NULL cfun and subsequently changing
>>
>> Why again? cfun should be set via push_cfun here so what's having cfun == NULL
>> at the pop_cfun point? Or rather, what code used set_cfun () instead
>> of push_cfun ()?
>>
>>>
>>> If you are fine with that, looks we've fixed all issues related to the change, right?
>>> Updated version of the is attached.
>>>
>>> Martin
>>>
>>>>
>>>>> Martin
>>>>>
>>>
>
> Hi Richard.
>
> There's the patch we talked about yesterday. It contains a few modification that
> were necessary to make it working:
>
> 1) cgraph_node::expand_thunk uses just set_cfun (NULL) & current_function_decl = NULL because
>
> bb = then_bb = else_bb = return_bb
> = init_lowered_empty_function (thunk_fndecl, true, count);
>
> in last branch of expand_thunk which calls allocate_struct_function.
>
> 2) i386.c and rs6000.c call init_function_start, as I move preamble to callers, I did the same
> for these two functions
>
> I've been running regression test and bootstrap.
Looks good to me.
Thanks,
Richard.
> Martin