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 <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)
