On Mon, 22 Oct 2018, David Malcolm wrote: > On Mon, 2018-10-22 at 15:56 +0200, Richard Biener wrote: > [...snip...] > > > This is what I finally applied for the original patch after fixing > > the above issue. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. > > > > Richard. > > > > 2018-10-22 Richard Biener <rguent...@suse.de> > > > > * gimple-ssa-evrp-analyze.c > > (evrp_range_analyzer::record_ranges_from_incoming_edge): Be > > smarter about what ranges to use. > > * tree-vrp.c (add_assert_info): Dump here. > > (register_edge_assert_for_2): Instead of here at multiple but > > not all places. > [...snip...] > > > Index: gcc/tree-vrp.c > > =================================================================== > > --- gcc/tree-vrp.c (revision 265381) > > +++ gcc/tree-vrp.c (working copy) > > @@ -2299,6 +2299,9 @@ add_assert_info (vec<assert_info> &asser > > info.val = val; > > info.expr = expr; > > asserts.safe_push (info); > > + dump_printf (MSG_NOTE | MSG_PRIORITY_INTERNALS, > > + "Adding assert for %T from %T %s %T\n", > > + name, expr, op_symbol_code (comp_code), val); > > } > > I think this dump_printf call needs to be wrapped in: > if (dump_enabled_p ()) > since otherwise it does non-trivial work, which is then discarded for > the common case where dumping is disabled. > > Alternatively, should dump_printf and dump_printf_loc test have an > early-reject internally for that?
Oh, I thought it had one - at least the "old" implementation did nothing expensive so if (dump_enabled_p ()) was just used to guard multiple printf stmts, avoiding multiple no-op calls. Did you check that all existing dump_* calls are wrapped inside a dump_enabled_p () region? If so I can properly guard the above. Otherwise I think we should restore previous expectation? Richard. > > > /* If NAME doesn't have an ASSERT_EXPR registered for asserting > > @@ -2698,16 +2701,6 @@ register_edge_assert_for_2 (tree name, e > > tmp = build1 (NOP_EXPR, TREE_TYPE (name), name3); > > if (cst2 != NULL_TREE) > > tmp = build2 (PLUS_EXPR, TREE_TYPE (name), tmp, cst2); > > - > > - if (dump_file) > > - { > > - fprintf (dump_file, "Adding assert for "); > > - print_generic_expr (dump_file, name3); > > - fprintf (dump_file, " from "); > > - print_generic_expr (dump_file, tmp); > > - fprintf (dump_file, "\n"); > > - } > > - > > add_assert_info (asserts, name3, tmp, comp_code, val); > > } > [...snip...] > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)