Re: Discover nothorow functions before into_ssa

[Richard Biener <rguenther at suse dot de>]

On Thu, 26 Mar 2015, Jan Hubicka wrote:

> Hi,
> this patch (as suggested by Richard) adds very simple discovery of
> DECL_NOTHORW to build_ssa passes.
> 
> The reason is that in 4.9 we did build_ssa in parallel with early optimization
> that does nothrow discovery as part of local pure const.  Bounds checking
> patches broke the pass queue into multiple lasses and we produce a lot more
> statements when notrhow is not identifier early.
> 
> I went with specialized pass because I do not want to pay the cost of
> local pure const building loop structure to prove that function is pure/const.
> We really care about this just later. I also tested a variant making this pass
> part of early lowering passes.  This does not work that well, because these
> are not run in topological order and C++ FE already does its own nothrow
> discovery, so it handled only about 900 calls on tramp3d.
> This pass handles about 3500 calls and additional 3000 calls are handled
> by the followup ipa-pure-const pass (probably because of extra code removal).
> 
> Adding pass causes cgraph verifier to fail.  The reason is that now fixup_cfg
> pass at begging of ssa passes actually does some dead code removal. This
> makes cgraph edges out of date and they are not rebuild at the end of the
> passes.
> 
> Instead of triggering yet another rebuild, which would be somewhat redundant
> given that early passes rebuilds the edges again, I just changed cgraph
> verifier to not compare calleers frequencies, but do callees.  This way
> we reduce some work, too.

Does that work on callgraph cycles?

> 
> Doing this I removed one very old FIXME about verificatoin that pointed out
> latent bug in set_edge_predicate. Fixed thus.

Can you commit this separately please?
 
> Bootstrapped/regtested x86_64-linux, OK?

See below.

> 	* cgraph.c (cgraph_edge::verify_count_and_frequency): Remove testing
> 	of frequency and bb match.
> 	(cgraph_node::verify_node): Do it here on callees only.
> 	* passes.def: Add pass_nothrow.
> 	* ipa-pure-const.c: (pass_data_nothrow): New.
> 	(pass_nothrow): New.
> 	(pass_nothrow::execute): New.
> 	(make_pass_nothrow): New.
> 	* tree-pass.h (make_pass_nothrow): Declare.
> 	* ipa-inline.c (set_edge_predicate): Also redirect indirect
> 	edges.
> Index: cgraph.c
> ===================================================================
> --- cgraph.c	(revision 221682)
> +++ cgraph.c	(working copy)
> @@ -2661,25 +2661,6 @@ cgraph_edge::verify_count_and_frequency
>        error ("caller edge frequency is too large");
>        error_found = true;
>      }
> -  if (gimple_has_body_p (caller->decl)
> -      && !caller->global.inlined_to
> -      && !speculative
> -      /* FIXME: Inline-analysis sets frequency to 0 when edge is optimized out.
> -	 Remove this once edges are actually removed from the function at that time.  */
> -      && (frequency
> -	  || (inline_edge_summary_vec.exists ()
> -	      && ((inline_edge_summary_vec.length () <= (unsigned) uid)
> -	          || !inline_edge_summary (this)->predicate)))
> -      && (frequency
> -	  != compute_call_stmt_bb_frequency (caller->decl,
> -					     gimple_bb (call_stmt))))
> -    {
> -      error ("caller edge frequency %i does not match BB frequency %i",
> -	     frequency,
> -	     compute_call_stmt_bb_frequency (caller->decl,
> -					     gimple_bb (call_stmt)));
> -      error_found = true;
> -    }
>    return error_found;
>  }
>  
> @@ -2848,9 +2829,46 @@ cgraph_node::verify_node (void)
>  	    error_found = true;
>  	  }
>      }
> +  for (e = callees; e; e = e->next_callee)
> +    {
> +      if (e->verify_count_and_frequency ())
> +	error_found = true;
> +      if (gimple_has_body_p (e->caller->decl)
> +	  && !e->caller->global.inlined_to
> +	  && !e->speculative
> +	  /* Optimized out calls are redirected to __builtin_unreachable.  */
> +	  && (e->frequency
> +	      || e->callee->decl
> +		 != builtin_decl_implicit (BUILT_IN_UNREACHABLE))
> +	  && (e->frequency
> +	      != compute_call_stmt_bb_frequency (e->caller->decl,
> +						 gimple_bb (e->call_stmt))))
> +	{
> +	  error ("caller edge frequency %i does not match BB frequency %i",
> +		 e->frequency,
> +		 compute_call_stmt_bb_frequency (e->caller->decl,
> +						 gimple_bb (e->call_stmt)));
> +	  error_found = true;
> +	}
> +    }
>    for (e = indirect_calls; e; e = e->next_callee)
> -    if (e->verify_count_and_frequency ())
> -      error_found = true;
> +    {
> +      if (e->verify_count_and_frequency ())
> +	error_found = true;
> +      if (gimple_has_body_p (e->caller->decl)
> +	  && !e->caller->global.inlined_to
> +	  && !e->speculative
> +	  && (e->frequency
> +	      != compute_call_stmt_bb_frequency (e->caller->decl,
> +						 gimple_bb (e->call_stmt))))
> +	{
> +	  error ("caller edge frequency %i does not match BB frequency %i",
> +		 e->frequency,
> +		 compute_call_stmt_bb_frequency (e->caller->decl,
> +						 gimple_bb (e->call_stmt)));
> +	  error_found = true;
> +	}
> +    }

This should be committed separately.

>    if (!callers && global.inlined_to)
>      {
>        error ("inlined_to pointer is set but no predecessors found");
> Index: passes.def
> ===================================================================
> --- passes.def	(revision 221682)
> +++ passes.def	(working copy)
> @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.
>        NEXT_PASS (pass_build_ssa);
>        NEXT_PASS (pass_ubsan);
>        NEXT_PASS (pass_early_warn_uninitialized);
> +      NEXT_PASS (pass_nothrow);
>    POP_INSERT_PASSES ()
>  
>    NEXT_PASS (pass_chkp_instrumentation_passes);
> Index: ipa-pure-const.c
> ===================================================================
> --- ipa-pure-const.c	(revision 221682)
> +++ ipa-pure-const.c	(working copy)
> @@ -1870,3 +1881,90 @@ make_pass_warn_function_noreturn (gcc::c
>  {
>    return new pass_warn_function_noreturn (ctxt);
>  }
> +
> +/* Simple local pass for pure const discovery reusing the analysis from
> +   ipa_pure_const.   This pass is effective when executed together with
> +   other optimization passes in early optimization pass queue.  */
> +
> +namespace {
> +
> +const pass_data pass_data_nothrow =
> +{
> +  GIMPLE_PASS, /* type */
> +  "nothrow", /* name */
> +  OPTGROUP_NONE, /* optinfo_flags */
> +  TV_IPA_PURE_CONST, /* tv_id */
> +  0, /* properties_required */
> +  0, /* properties_provided */
> +  0, /* properties_destroyed */
> +  0, /* todo_flags_start */
> +  0, /* todo_flags_finish */
> +};
> +
> +class pass_nothrow : public gimple_opt_pass
> +{
> +public:
> +  pass_nothrow (gcc::context *ctxt)
> +    : gimple_opt_pass (pass_data_nothrow, ctxt)
> +  {}
> +
> +  /* opt_pass methods: */
> +  opt_pass * clone () { return new pass_nothrow (m_ctxt); }
> +  virtual bool gate (function *) { return optimize; }
> +  virtual unsigned int execute (function *);
> +
> +}; // class pass_nothrow
> +
> +unsigned int
> +pass_nothrow::execute (function *)
> +{
> +  struct cgraph_node *node;
> +  basic_block this_block;
> +
> +  if (TREE_NOTHROW (current_function_decl))
> +    return 0;
> +
> +  node = cgraph_node::get (current_function_decl);
> +
> +  /* We run during lowering, we can not really use availability yet.  */
> +  if (cgraph_node::get (current_function_decl)->get_availability ()
> +      <= AVAIL_INTERPOSABLE)
> +    {
> +      if (dump_file)
> +        fprintf (dump_file, "Function is interposable;"
> +	         " not analyzing.\n");
> +      return true;
> +    }
> +
> +  FOR_EACH_BB_FN (this_block, cfun)
> +    {
> +      gimple_stmt_iterator gsi;
> +      for (gsi = gsi_start_bb (this_block);
> +	   !gsi_end_p (gsi);
> +	   gsi_next (&gsi))
> +        if (stmt_could_throw_p (gsi_stmt (gsi))
> +	    && stmt_can_throw_external (gsi_stmt (gsi)))

The stmt_could_throw_p call is redundant.

> +	  {
> +	    if (dump_file)
> +	      {
> +		fprintf (dump_file, "Statement can throw: ");
> +		print_gimple_stmt (dump_file, gsi_stmt (gsi), 0, 0);
> +	      }
> +	    return 0;
> +	  }
> +    }
> +
> +  node->set_nothrow_flag (true);
> +  if (dump_file)
> +    fprintf (dump_file, "Function found to be nothrow: %s\n",
> +	     current_function_name ());
> +  return execute_fixup_cfg ();

Err - this doesn't make sense - you'd need to call this for
all _callers_.  That is, this is dealt with by

  PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
      NEXT_PASS (pass_fixup_cfg);
^^^^^^^

?  If not then this may paper over the issue I saw (dangling
call_stmt on a cgraph edge?).

So please re-check with the above execute_fixup_cfg removed
(your simple patch doesn't handle direct recursion optimistically).

Thanks,
Richard.

> +}
> +
> +} // anon namespace
> +
> +gimple_opt_pass *
> +make_pass_nothrow (gcc::context *ctxt)
> +{
> +  return new pass_nothrow (ctxt);
> +}
> Index: tree-pass.h
> ===================================================================
> --- tree-pass.h	(revision 221682)
> +++ tree-pass.h	(working copy)
> @@ -436,6 +436,7 @@ extern gimple_opt_pass *make_pass_remove
>  							      *ctxt);
>  extern gimple_opt_pass *make_pass_build_cgraph_edges (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_local_pure_const (gcc::context *ctxt);
> +extern gimple_opt_pass *make_pass_nothrow (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_tracer (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_warn_unused_result (gcc::context *ctxt);
>  extern gimple_opt_pass *make_pass_diagnose_tm_blocks (gcc::context *ctxt);
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c	(revision 221682)
> +++ ipa-inline-analysis.c	(working copy)
> @@ -769,11 +769,16 @@ edge_set_predicate (struct cgraph_edge *
>  
>    /* If the edge is determined to be never executed, redirect it
>       to BUILTIN_UNREACHABLE to save inliner from inlining into it.  */
> -  if (predicate && false_predicate_p (predicate) && e->callee)
> +  if (predicate && false_predicate_p (predicate))
>      {
>        struct cgraph_node *callee = !e->inline_failed ? e->callee : NULL;
> -
> -      e->redirect_callee (cgraph_node::get_create
> +      if (e->speculative)
> +	e->resolve_speculation (builtin_decl_implicit (BUILT_IN_UNREACHABLE));
> +      if (!e->callee)
> +	e->make_direct (cgraph_node::get_create
> +			  (builtin_decl_implicit (BUILT_IN_UNREACHABLE)));
> +      else
> +	e->redirect_callee (cgraph_node::get_create
>  			    (builtin_decl_implicit (BUILT_IN_UNREACHABLE)));
>        e->inline_failed = CIF_UNREACHABLE;
>        es->call_stmt_size = 0;
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)