Set current_function_decl in {push,pop}_cfun and push_struct_function

Hi,

On Wed, Aug 22, 2012 at 03:37:48PM +0200, Richard Guenther wrote:
> On Wed, Aug 22, 2012 at 3:04 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > On Tue, Aug 21, 2012 at 01:30:47PM +0200, Richard Guenther wrote:
> >> On Tue, Aug 21, 2012 at 1:27 PM, Martin Jambor <mjambor@suse.cz> wrote:
> >> > On Wed, Aug 15, 2012 at 05:21:04PM +0200, Martin Jambor wrote:
> >> >> Hi,
> >> >>
> >> >> On Fri, Aug 10, 2012 at 04:57:41PM +0200, Eric Botcazou wrote:
> >> >> > > - ada/gcc-interface/utils.c:rest_of_subprog_body_compilation calls
> >> >> > >   dump_function which in turns calls dump_function_to_file which calls
> >> >> > >   push_cfun.  But Ada front end has its idea of the
> >> >> > >   current_function_decl and there is no cfun which is an inconsistency
> >> >> > >   which makes push_cfun assert fail.  I "solved" it by temporarily
> >> >> > >   setting current_function_decl to NULL_TREE.  It's just dumping and I
> >> >> > >   thought that dump_function should be considered middle-end and thus
> >> >> > >   middle-end invariants should apply.
> >> >> >
> >> >> > If you think that calling dump_function from rest_of_subprog_body_compilation
> >> >> > is a layering violation, I don't have a problem with replacing it with a more
> >> >> > "manual" scheme like the one in c-family/c-gimplify.c:c_genericize, provided
> >> >> > that this yields roughly the same output.
> >> >>
> >> >> Richi suggested on IRC that I remove the push/pop_cfun calls from
> >> >> dump_function_to_file.  The only problem seems to be
> >> >> dump_histograms_for_stmt
> >> >
> >> > Yesterday I actually tried and it is not the only problem.  Another
> >> > one is dump_function_to_file->dump_bb->maybe_hot_bb_p which uses cfun
> >> > to read profile_status.  There may be others, this one just blew up
> >> > first when I set cfun to NULL.  And in future someone is quite likely
> >> > to need cfun to dump something new too.
> >> >
> >> > At the same time, re-implementing dumping
> >> > c-family/c-gimplify.c:c_genericize when dump_function suffices seems
> >> > ugly to me.
> >> >
> >> > So I am going to declare dump_function a front-end interface and use
> >> > set_cfun in my original patch in dump_function_to_file like we do in
> >> > other such functions.
> >> >
> >> > I hope that will be OK.  Thanks,
> >>
> >> Setting cfun has side-effects of switching target stuff which might have
> >> code-generation side-effects because of implementation issues we have
> >> with target/optimize attributes.  So I don't think cfun should be changed
> >> just for dumping.
> >>
> >> Can you instead just set current_function_decl and access
> >> struct function via DECL_STRUCT_FUNCTION in the dumpers then?
> >> After all, it it is a front-end interface, the frontend way of saying
> >> "this is the current function" is to set current_function_decl, not the
> >> middle-end cfun.
> >>
> >
> > Like the following?  Tested and bootstrapped on x86_64-linux, it does
> > help avoid the ada hunk in my previous patch.  OK for trunk?
> 
> Ok if nobody complains - btw, I think you miss to restore current_function_decl
> here:
> 
> 
> > *** /tmp/HcgoTd_tree-cfg.c      Wed Aug 22 15:02:30 2012
> > --- gcc/tree-cfg.c      Wed Aug 22 11:53:02 2012
> > *************** move_sese_region_to_fn (struct function
> > *** 6632,6650 ****
> >      */
> >
> >   void
> > ! dump_function_to_file (tree fn, FILE *file, int flags)
> >   {
> > !   tree arg, var;
> >     struct function *dsf;
> >     bool ignore_topmost_bind = false, any_var = false;
> >     basic_block bb;
> >     tree chain;
> > !   bool tmclone = TREE_CODE (fn) == FUNCTION_DECL && decl_is_tm_clone (fn);
> >
> > !   fprintf (file, "%s %s(", current_function_name (),
> > !          tmclone ? "[tm-clone] " : "");
> >
> > !   arg = DECL_ARGUMENTS (fn);
> >     while (arg)
> >       {
> >         print_generic_expr (file, TREE_TYPE (arg), dump_flags);
> > --- 6632,6652 ----
> >      */
> >
> >   void
> > ! dump_function_to_file (tree fndecl, FILE *file, int flags)
> >   {
> > !   tree arg, var, old_current_fndecl = current_function_decl;
> >     struct function *dsf;
> >     bool ignore_topmost_bind = false, any_var = false;
> >     basic_block bb;
> >     tree chain;
> > !   bool tmclone = (TREE_CODE (fndecl) == FUNCTION_DECL
> > !                 && decl_is_tm_clone (fndecl));
> > !   struct function *fun = DECL_STRUCT_FUNCTION (fndecl);
> >
> > !   current_function_decl = fndecl;
> > !   fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : "");
> >
> > !   arg = DECL_ARGUMENTS (fndecl);
> >     while (arg)
> >       {
> >         print_generic_expr (file, TREE_TYPE (arg), dump_flags);
> > *************** dump_function_to_file (tree fn, FILE *fi
> > *** 6659,6689 ****
> >     fprintf (file, ")\n");
> >
> >     if (flags & TDF_VERBOSE)
> > !     print_node (file, "", fn, 2);
> >
> > !   dsf = DECL_STRUCT_FUNCTION (fn);
> >     if (dsf && (flags & TDF_EH))
> >       dump_eh_tree (file, dsf);
> >
> > !   if (flags & TDF_RAW && !gimple_has_body_p (fn))
> >       {
> > !       dump_node (fn, TDF_SLIM | flags, file);
> >         return;
> >       }
> >
> > -   /* Switch CFUN to point to FN.  */
> > -   push_cfun (DECL_STRUCT_FUNCTION (fn));
> > -
> >     /* When GIMPLE is lowered, the variables are no longer available in
> >        BIND_EXPRs, so display them separately.  */
> > !   if (cfun && cfun->decl == fn && (cfun->curr_properties & PROP_gimple_lcf))
> >       {
> >         unsigned ix;
> >         ignore_topmost_bind = true;
> >
> >         fprintf (file, "{\n");
> > !       if (!VEC_empty (tree, cfun->local_decls))
> > !       FOR_EACH_LOCAL_DECL (cfun, ix, var)
> >           {
> >             print_generic_decl (file, var, flags);
> >             if (flags & TDF_VERBOSE)
> > --- 6661,6688 ----
> >     fprintf (file, ")\n");
> >
> >     if (flags & TDF_VERBOSE)
> > !     print_node (file, "", fndecl, 2);
> >
> > !   dsf = DECL_STRUCT_FUNCTION (fndecl);
> >     if (dsf && (flags & TDF_EH))
> >       dump_eh_tree (file, dsf);
> >
> > !   if (flags & TDF_RAW && !gimple_has_body_p (fndecl))
> >       {
> > !       dump_node (fndecl, TDF_SLIM | flags, file);
> >         return;
> 
> ^^^^
> 
> and maybe other early exits from dump_function_to_file.
> 

Oh, I missed that, there are no further early returns in the function.
I have added another restoration of current_function_decl there and
committed the patch below after retesting.

> Also as said on IRC when you switch current_function_decl you should be
> able to set cfun to NULL (and restore it afterwards, without invoking the
> target hook for cfun switching).  Thus, encapsulating this into a
> push/pop_current_function_decl would be nice.

Good idea, I will incorporate that into my original patch.

Thanks,

Martin

2012-08-24  Martin Jambor  <mjambor@suse.cz>

	* predict.c (maybe_hot_frequency_p): New parameter fun.  Use its decl
	instead of current_function_decl, use profile_status_for_function and
	ENTRY_BLOCK_PTR_FOR_FUNCTION with fun instead of their cfun variants.
	(maybe_hot_count_p): New parameter fun, use
	profile_status_for_function instead of its cfun_variant.
	(maybe_hot_bb_p): New parameter fun, checking-assert it, pass it to
	all callees.
	(maybe_hot_edge_p): Pass cfun to maybe_hot_count_p and
	maybe_hot_frequency_p.
	(probably_never_executed_bb_p): New parameter fun, use its decl
	instead of current_function_decl.
	(optimize_bb_for_size_p): Pass cfun to maybe_hot_bb_p.
	(rtl_profile_for_bb): Likewise.
	(compute_function_frequency): Pass cfun to maybe_hot_bb_p and
	probably_never_executed_bb_p.
	* tree-ssa-operands.c (ssa_operands_active): New operator fun.  Use it
	instead of cfun.
	(update_stmt_operands): Pass cfun as an argument of
	ssa_operands_active.
	(swap_tree_operands): Likewise.
	* gimple-iterator.c (update_modified_stmt): Likewise.
	(update_modified_stmts): Likewise.
	* tree-flow-inline.h (delink_stmt_imm_use): Likewise.
	* tree-ssa.c (delete_tree_ssa): Likewise.
	* bb-reorder.c (bb_to_key): Pass cfun to probably_never_executed_bb_p.
	(push_to_next_round_p): Likewise.
	(find_rarely_executed_basic_blocks_and_crossing_edges ): Likewise.
	* cfg.c: Inlude tree.h.
	(check_bb_profile): Use profile_status_for_function,
	EXIT_BLOCK_PTR_FOR_FUNCTION and ENTRY_BLOCK_PTR_FOR_FUNCTION with
	DECL_STRUCT_FUNCTION (current_function_decl) instead of their cfun
	variants.
	(dump_bb_info): Pass DECL_STRUCT_FUNCTION (current_function_decl) to
	maybe_hot_bb_p and probably_never_executed_bb_p.
	* gimple-pretty-print.c (gimple_dump_bb_buff): Checking-assert that
	DECL_STRUCT_FUNCTION (current_function_decl) is not NULL.  Pass it to
	dump_histograms_for_stmt.
	(dump_gimple_mem_ops): Pass DECL_STRUCT_FUNCTION (current_function_decl)
	as an argument to dump_gimple_mem_ops.
	* tree-cfg.c (dump_function_to_file): Rename parameter fn to fndecl.
	Do not change cfun.  Change and restore current_function_decl.
	* Makefile.in (cfg.o): Include TREE_H in dependencies.

Set current_function_decl in {push,pop}_cfun and push_struct_function

Commit Message

Patch