This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix some EVRP stupidness
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 <rguenther@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)