On Wed, Oct 21, 2015 at 1:24 PM, Martin Liška <mli...@suse.cz> wrote:
> On 10/21/2015 11:59 AM, Richard Biener wrote:
>> On Wed, Oct 21, 2015 at 11:19 AM, Martin Liška <mli...@suse.cz> wrote:
>>> On 10/20/2015 03:39 PM, Richard Biener wrote:
>>>> On Tue, Oct 20, 2015 at 3:00 PM, Martin Liška <mli...@suse.cz> wrote:
>>>>> Hello.
>>>>>
>>>>> As part of upcoming merge of HSA branch, we would like to have 
>>>>> possibility to terminate
>>>>> pass manager after execution of the HSA generation pass. The HSA back-end 
>>>>> is implemented
>>>>> as a tree pass that directly emits HSAIL from gimple tree representation. 
>>>>> The pass operates
>>>>> on clones created by HSA IPA pass and the pass manager should stop 
>>>>> execution of further
>>>>> RTL passes.
>>>>>
>>>>> Suggested patch survives bootstrap and regression tests on 
>>>>> x86_64-linux-pc.
>>>>>
>>>>> What do you think about it?
>>>>
>>>> Are you sure it works this way?
>>>>
>>>> Btw, you will miss executing of all the cleanup passes that will
>>>> eventually free memory
>>>> associated with the function.  So I'd rather support a
>>>> TODO_discard_function which
>>>> should basically release the body from the cgraph.
>>>
>>> Hi.
>>>
>>> Agree with you that I should execute all TODOs, which can be easily done.
>>> However, if I just try to introduce the suggested TODO and handle it 
>>> properly
>>> by calling cgraph_node::release_body, then for instance fn->gimple_df, 
>>> fn->cfg are
>>> released and I hit ICEs on many places.
>>>
>>> Stopping the pass manager looks necessary, or do I miss something?
>>
>> "Stopping the pass manager" is necessary after TODO_discard_function, yes.
>> But that may be simply done via a has_body () check then?
>
> Thanks, there's second version of the patch. I'm going to start regression 
> tests.

As release_body () will free cfun you should pop_cfun () before
calling it (and thus
drop its modification).  Also TODO_discard_functiuon should be only set for
local passes thus you should probably add a gcc_assert (cfun).
I'd move its handling earlier, definitely before the ggc_collect, eventually
before the pass_fini_dump_file () (do we want a last dump of the
function or not?).

@@ -2397,6 +2410,10 @@ execute_pass_list_1 (opt_pass *pass)
     {
       gcc_assert (pass->type == GIMPLE_PASS
                  || pass->type == RTL_PASS);
+
+
+      if (!gimple_has_body_p (current_function_decl))
+       return;

too much vertical space.  With popping cfun before releasing the body the check
might just become if (!cfun) and

@@ -2409,7 +2426,7 @@ execute_pass_list (function *fn, opt_pass *pass)
 {
   push_cfun (fn);
   execute_pass_list_1 (pass);
-  if (fn->cfg)
+  if (gimple_has_body_p (current_function_decl) && fn->cfg)
     {
       free_dominance_info (CDI_DOMINATORS);
       free_dominance_info (CDI_POST_DOMINATORS);

here you'd need to guard the pop_cfun call on cfun != NULL.  IMHO it's better
to not let cfun point to deallocated data.

Otherwise looks good to me.

Richard.


> Martin
>
>>
>>> Thanks,
>>> Martin
>>>
>>>>
>>>> Richard.
>>>>
>>>>> Thanks,
>>>>> Martin
>>>
>

Reply via email to