| Message ID | alpine.LSU.2.20.1810181248510.4374@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Fix some EVRP stupidness | expand |
On Thu, 18 Oct 2018, Richard Biener wrote: > > At some point we decided to not simply intersect all ranges we get > via register_edge_assert_for. Instead we simply register them > in-order. That causes things like replacing [64, +INF] with ~[0, 0]. > > The following patch avoids replacing a range with a larger one > as obvious improvement. > > Compared to assert_expr based VRP we lack the ability to put down > actual assert_exprs and thus multiple SSA names with ranges we > could link via equivalences. In the end we need sth similar, > for example by keeping a stack of active ranges for each SSA name. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Actually not. Needed to update to the new value_range class and after that (and its introduction of ->check()) we now ICE during bootstrap with during GIMPLE pass: evrp /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c: In function ‘get_BID128’: /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c:1851:1: internal compiler error: in check, at tree-vrp.c:155 1851 | } | ^ 0xf3a8b5 value_range::check() /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:155 0xf42424 value_range::value_range(value_range_kind, tree_node*, tree_node*, bitmap_head*) /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:110 0xf42424 set_value_range_with_overflow /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1422 0xf42424 extract_range_from_binary_expr_1(value_range*, tree_code, tree_node*, value_range const*, value_range const*) /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1679 for a PLUS_EXPR of [12254, expon_43] and [-1, -1] yielding (temporarily!) [12254, -1] before supposed to be adjusted by the symbolic bound: /* Adjust the range for possible overflow. */ set_value_range_with_overflow (*vr, expr_type, wmin, wmax, min_ovf, max_ovf); ^^^ ICE if (vr->varying_p ()) return; /* Build the symbolic bounds if needed. */ min = vr->min (); max = vr->max (); adjust_symbolic_bound (min, code, expr_type, sym_min_op0, sym_min_op1, neg_min_op0, neg_min_op1); adjust_symbolic_bound (max, code, expr_type, sym_max_op0, sym_max_op1, neg_max_op0, neg_max_op1); type = vr->kind (); I think the refactoring that was applied here is simply not suitable because *vr is _not_ necessarily a valid range before the symbolic bounds have been adjusted. A fix would be sth like the following which I am going to test now. Richard. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 40d40e5e2fe..c5748a43246 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -1328,7 +1328,7 @@ combine_bound (enum tree_code code, wide_int &wi, wi::overflow_type &ovf, underflow. +1 indicates overflow. 0 indicates neither. */ static void -set_value_range_with_overflow (value_range &vr, +set_value_range_with_overflow (value_range_kind &kind, tree &min, tree &max, tree type, const wide_int &wmin, const wide_int &wmax, wi::overflow_type min_ovf, @@ -1341,7 +1341,7 @@ set_value_range_with_overflow (value_range &vr, range covers all values. */ if (prec == 1 && wi::lt_p (wmax, wmin, sgn)) { - set_value_range_to_varying (&vr); + kind = VR_VARYING; return; } @@ -1357,13 +1357,15 @@ set_value_range_with_overflow (value_range &vr, the entire range. We have a similar check at the end of extract_range_from_binary_expr_1. */ if (wi::gt_p (tmin, tmax, sgn)) - vr.set_varying (); + kind = VR_VARYING; else - /* No overflow or both overflow or underflow. The - range kind stays VR_RANGE. */ - vr = value_range (VR_RANGE, - wide_int_to_tree (type, tmin), - wide_int_to_tree (type, tmax)); + { + kind = VR_RANGE; + /* No overflow or both overflow or underflow. The + range kind stays VR_RANGE. */ + min = wide_int_to_tree (type, tmin); + max = wide_int_to_tree (type, tmax); + } return; } else if ((min_ovf == wi::OVF_UNDERFLOW && max_ovf == wi::OVF_NONE) @@ -1384,18 +1386,18 @@ set_value_range_with_overflow (value_range &vr, types values. */ if (covers || wi::cmp (tmin, tmax, sgn) > 0) { - set_value_range_to_varying (&vr); + kind = VR_VARYING; return; } - vr = value_range (VR_ANTI_RANGE, - wide_int_to_tree (type, tmin), - wide_int_to_tree (type, tmax)); + kind = VR_ANTI_RANGE; + min = wide_int_to_tree (type, tmin); + max = wide_int_to_tree (type, tmax); return; } else { /* Other underflow and/or overflow, drop to VR_VARYING. */ - set_value_range_to_varying (&vr); + kind = VR_VARYING; return; } } @@ -1405,7 +1407,7 @@ set_value_range_with_overflow (value_range &vr, value. */ wide_int type_min = wi::min_value (prec, sgn); wide_int type_max = wi::max_value (prec, sgn); - tree min, max; + kind = VR_RANGE; if (min_ovf == wi::OVF_UNDERFLOW) min = wide_int_to_tree (type, type_min); else if (min_ovf == wi::OVF_OVERFLOW) @@ -1419,7 +1421,6 @@ set_value_range_with_overflow (value_range &vr, max = wide_int_to_tree (type, type_max); else max = wide_int_to_tree (type, wmax); - vr = value_range (VR_RANGE, min, max); } } @@ -1676,21 +1677,20 @@ extract_range_from_binary_expr_1 (value_range *vr, } /* Adjust the range for possible overflow. */ - set_value_range_with_overflow (*vr, expr_type, + min = NULL_TREE; + max = NULL_TREE; + set_value_range_with_overflow (type, min, max, expr_type, wmin, wmax, min_ovf, max_ovf); - if (vr->varying_p ()) + if (type == VR_VARYING) return; /* Build the symbolic bounds if needed. */ - min = vr->min (); - max = vr->max (); adjust_symbolic_bound (min, code, expr_type, sym_min_op0, sym_min_op1, neg_min_op0, neg_min_op1); adjust_symbolic_bound (max, code, expr_type, sym_max_op0, sym_max_op1, neg_max_op0, neg_max_op1); - type = vr->kind (); } else { > Richard. > > 2018-10-18 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. > > * gcc.dg/tree-ssa/evrp12.c: New testcase. > * gcc.dg/predict-6.c: Adjust. > * gcc.dg/tree-ssa/vrp33.c: Disable EVRP. > * gcc.dg/tree-ssa/vrp02.c: Likewise. > * gcc.dg/tree-ssa/cunroll-9.c: Likewise. > > diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c > index e9afa80e191..0748a53cdb8 100644 > --- a/gcc/gimple-ssa-evrp-analyze.c > +++ b/gcc/gimple-ssa-evrp-analyze.c > @@ -206,6 +206,17 @@ evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb) > ordering issues that can lead to worse ranges. */ > for (unsigned i = 0; i < vrs.length (); ++i) > { > + /* But make sure we do not weaken ranges like when > + getting first [64, +INF] and then ~[0, 0] from > + conditions like (s & 0x3cc0) == 0). */ > + value_range *old_vr = get_value_range (vrs[i].first); > + value_range tem = *old_vr; > + tem.equiv = NULL; > + vrp_intersect_ranges (&tem, vrs[i].second); > + if (tem.type == old_vr->type > + && tem.min == old_vr->min > + && tem.max == old_vr->max) > + continue; > push_value_range (vrs[i].first, vrs[i].second); > if (is_fallthru > && all_uses_feed_or_dominated_by_stmt (vrs[i].first, stmt)) > diff --git a/gcc/testsuite/gcc.dg/predict-6.c b/gcc/testsuite/gcc.dg/predict-6.c > index 5d6fbf158f2..08ce5cdb81d 100644 > --- a/gcc/testsuite/gcc.dg/predict-6.c > +++ b/gcc/testsuite/gcc.dg/predict-6.c > @@ -10,9 +10,9 @@ void foo (int base, int bound) > int i, ret = 0; > for (i = base; i <= bound; i++) > { > - if (i < base) > + if (i <= base) > global += bar (i); > - if (i < base + 1) > + if (i < base + 2) > global += bar (i); > if (i <= base + 3) > global += bar (i); > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c b/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c > index 0e4407dcbd7..886dc147ad1 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-cunrolli-details" } */ > +/* { dg-options "-O2 -fdump-tree-cunrolli-details -fdisable-tree-evrp" } */ > void abort (void); > int q (void); > int a[10]; > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/evrp12.c b/gcc/testsuite/gcc.dg/tree-ssa/evrp12.c > new file mode 100644 > index 00000000000..b3906c23465 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/evrp12.c > @@ -0,0 +1,21 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-evrp" } */ > + > +extern void link_error (); > + > +void > +f3 (unsigned int s) > +{ > + if ((s & 0x3cc0) == 0) > + { > + if (s >= -15552U) > + link_error (); > + } > + else > + { > + if (s <= 0x3f) > + link_error (); > + } > +} > + > +/* { dg-final { scan-tree-dump-not "link_error" "evrp" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c > index 8d14feadb6a..4be538f5944 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks" } */ > +/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks -fdisable-tree-evrp" } */ > > struct A > { > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c > index 75fefa49925..f1d3863943e 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-vrp1 -fno-tree-fre" } */ > +/* { dg-options "-O2 -fdump-tree-vrp1 -fno-tree-fre -fdisable-tree-evrp" } */ > > /* This is from PR14052. */ > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > index cbc2ea2f26b..6f5ec43670e 100644 > --- a/gcc/tree-vrp.c > +++ b/gcc/tree-vrp.c > @@ -2133,6 +2133,17 @@ add_assert_info (vec<assert_info> &asserts, > info.val = val; > info.expr = expr; > asserts.safe_push (info); > + > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, "Adding assert for "); > + print_generic_expr (dump_file, name); > + fprintf (dump_file, " from "); > + print_generic_expr (dump_file, expr); > + fprintf (dump_file, " %s ", op_symbol_code (comp_code)); > + print_generic_expr (dump_file, val); > + fprintf (dump_file, "\n"); > + } > } > > /* If NAME doesn't have an ASSERT_EXPR registered for asserting > @@ -2532,16 +2543,6 @@ register_edge_assert_for_2 (tree name, edge 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); > } > > @@ -2559,16 +2560,6 @@ register_edge_assert_for_2 (tree name, edge e, > tmp = build1 (NOP_EXPR, TREE_TYPE (name), tmp); > 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, name2); > - fprintf (dump_file, " from "); > - print_generic_expr (dump_file, tmp); > - fprintf (dump_file, "\n"); > - } > - > add_assert_info (asserts, name2, tmp, comp_code, val); > } > } > @@ -2691,16 +2682,6 @@ register_edge_assert_for_2 (tree name, edge e, > cst = fold_build2 (MINUS_EXPR, TREE_TYPE (name2), cst, > build_int_cst (TREE_TYPE (name2), 1)); > } > - > - if (dump_file) > - { > - fprintf (dump_file, "Adding assert for "); > - print_generic_expr (dump_file, name2); > - fprintf (dump_file, " from "); > - print_generic_expr (dump_file, tmp); > - fprintf (dump_file, "\n"); > - } > - > add_assert_info (asserts, name2, tmp, new_comp_code, cst); > } > } > @@ -2765,18 +2746,7 @@ register_edge_assert_for_2 (tree name, edge e, > } > > if (new_val) > - { > - if (dump_file) > - { > - fprintf (dump_file, "Adding assert for "); > - print_generic_expr (dump_file, name2); > - fprintf (dump_file, " from "); > - print_generic_expr (dump_file, tmp); > - fprintf (dump_file, "\n"); > - } > - > - add_assert_info (asserts, name2, tmp, new_comp_code, new_val); > - } > + add_assert_info (asserts, name2, tmp, new_comp_code, new_val); > } > > /* Add asserts for NAME cmp CST and NAME being defined as > @@ -3004,16 +2974,6 @@ register_edge_assert_for_2 (tree name, edge e, > maxv2 = maxv - minv; > } > new_val = wide_int_to_tree (type, maxv2); > - > - if (dump_file) > - { > - fprintf (dump_file, "Adding assert for "); > - print_generic_expr (dump_file, names[i]); > - fprintf (dump_file, " from "); > - print_generic_expr (dump_file, tmp); > - fprintf (dump_file, "\n"); > - } > - > add_assert_info (asserts, names[i], tmp, LE_EXPR, new_val); > } > } >
On 10/18/18 8:11 AM, Richard Biener wrote: > On Thu, 18 Oct 2018, Richard Biener wrote: > >> >> At some point we decided to not simply intersect all ranges we get >> via register_edge_assert_for. Instead we simply register them >> in-order. That causes things like replacing [64, +INF] with ~[0, 0]. >> >> The following patch avoids replacing a range with a larger one >> as obvious improvement. >> >> Compared to assert_expr based VRP we lack the ability to put down >> actual assert_exprs and thus multiple SSA names with ranges we >> could link via equivalences. In the end we need sth similar, >> for example by keeping a stack of active ranges for each SSA name. >> >> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > Actually not. Needed to update to the new value_range class and after > that (and its introduction of ->check()) we now ICE during bootstrap > with > > during GIMPLE pass: evrp > /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c: In > function ‘get_BID128’: > /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c:1851:1: > internal compiler error: in check, at tree-vrp.c:155 > 1851 | } > | ^ > 0xf3a8b5 value_range::check() > /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:155 > 0xf42424 value_range::value_range(value_range_kind, tree_node*, > tree_node*, bitmap_head*) > /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:110 > 0xf42424 set_value_range_with_overflow > /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1422 > 0xf42424 extract_range_from_binary_expr_1(value_range*, tree_code, > tree_node*, value_range const*, value_range const*) > /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1679 > > for a PLUS_EXPR of [12254, expon_43] and [-1, -1] yielding > (temporarily!) [12254, -1] before supposed to be adjusted by the > symbolic bound: > > /* Adjust the range for possible overflow. */ > set_value_range_with_overflow (*vr, expr_type, > wmin, wmax, min_ovf, max_ovf); > ^^^ ICE > if (vr->varying_p ()) > return; > > /* Build the symbolic bounds if needed. */ > min = vr->min (); > max = vr->max (); > adjust_symbolic_bound (min, code, expr_type, > sym_min_op0, sym_min_op1, > neg_min_op0, neg_min_op1); > adjust_symbolic_bound (max, code, expr_type, > sym_max_op0, sym_max_op1, > neg_max_op0, neg_max_op1); > type = vr->kind (); > > I think the refactoring that was applied here is simply not suitable > because *vr is _not_ necessarily a valid range before the symbolic > bounds have been adjusted. A fix would be sth like the following > which I am going to test now. Sounds reasonable. Is this PR87640? Because the testcase there is also crashing while creating the range right before adjusting the symbolics. Thanks for looking at this. Aldy
On October 18, 2018 5:42:56 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote: > > >On 10/18/18 8:11 AM, Richard Biener wrote: >> On Thu, 18 Oct 2018, Richard Biener wrote: >> >>> >>> At some point we decided to not simply intersect all ranges we get >>> via register_edge_assert_for. Instead we simply register them >>> in-order. That causes things like replacing [64, +INF] with ~[0, >0]. >>> >>> The following patch avoids replacing a range with a larger one >>> as obvious improvement. >>> >>> Compared to assert_expr based VRP we lack the ability to put down >>> actual assert_exprs and thus multiple SSA names with ranges we >>> could link via equivalences. In the end we need sth similar, >>> for example by keeping a stack of active ranges for each SSA name. >>> >>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to >trunk. >> >> Actually not. Needed to update to the new value_range class and >after >> that (and its introduction of ->check()) we now ICE during bootstrap >> with >> >> during GIMPLE pass: evrp >> /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c: In >> function ‘get_BID128’: >> >/space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c:1851:1: >> internal compiler error: in check, at tree-vrp.c:155 >> 1851 | } >> | ^ >> 0xf3a8b5 value_range::check() >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:155 >> 0xf42424 value_range::value_range(value_range_kind, tree_node*, >> tree_node*, bitmap_head*) >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:110 >> 0xf42424 set_value_range_with_overflow >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1422 >> 0xf42424 extract_range_from_binary_expr_1(value_range*, tree_code, >> tree_node*, value_range const*, value_range const*) >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1679 >> >> for a PLUS_EXPR of [12254, expon_43] and [-1, -1] yielding >> (temporarily!) [12254, -1] before supposed to be adjusted by the >> symbolic bound: >> >> /* Adjust the range for possible overflow. */ >> set_value_range_with_overflow (*vr, expr_type, >> wmin, wmax, min_ovf, >max_ovf); >> ^^^ ICE >> if (vr->varying_p ()) >> return; >> >> /* Build the symbolic bounds if needed. */ >> min = vr->min (); >> max = vr->max (); >> adjust_symbolic_bound (min, code, expr_type, >> sym_min_op0, sym_min_op1, >> neg_min_op0, neg_min_op1); >> adjust_symbolic_bound (max, code, expr_type, >> sym_max_op0, sym_max_op1, >> neg_max_op0, neg_max_op1); >> type = vr->kind (); >> >> I think the refactoring that was applied here is simply not suitable >> because *vr is _not_ necessarily a valid range before the symbolic >> bounds have been adjusted. A fix would be sth like the following >> which I am going to test now. > >Sounds reasonable. Doesn't work and miscompiles all over the place. >Is this PR87640? Because the testcase there is also crashing while >creating the range right before adjusting the symbolics. Might be. I'll poke some more tomorrow unless you beat me to it. Richard. >Thanks for looking at this. >Aldy
On Thu, 18 Oct 2018, Richard Biener wrote: > On October 18, 2018 5:42:56 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote: > > > > > >On 10/18/18 8:11 AM, Richard Biener wrote: > >> On Thu, 18 Oct 2018, Richard Biener wrote: > >> > >>> > >>> At some point we decided to not simply intersect all ranges we get > >>> via register_edge_assert_for. Instead we simply register them > >>> in-order. That causes things like replacing [64, +INF] with ~[0, > >0]. > >>> > >>> The following patch avoids replacing a range with a larger one > >>> as obvious improvement. > >>> > >>> Compared to assert_expr based VRP we lack the ability to put down > >>> actual assert_exprs and thus multiple SSA names with ranges we > >>> could link via equivalences. In the end we need sth similar, > >>> for example by keeping a stack of active ranges for each SSA name. > >>> > >>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to > >trunk. > >> > >> Actually not. Needed to update to the new value_range class and > >after > >> that (and its introduction of ->check()) we now ICE during bootstrap > >> with > >> > >> during GIMPLE pass: evrp > >> /space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c: In > >> function ‘get_BID128’: > >> > >/space/rguenther/src/svn/trunk/libgcc/config/libbid/bid128_div.c:1851:1: > >> internal compiler error: in check, at tree-vrp.c:155 > >> 1851 | } > >> | ^ > >> 0xf3a8b5 value_range::check() > >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:155 > >> 0xf42424 value_range::value_range(value_range_kind, tree_node*, > >> tree_node*, bitmap_head*) > >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:110 > >> 0xf42424 set_value_range_with_overflow > >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1422 > >> 0xf42424 extract_range_from_binary_expr_1(value_range*, tree_code, > >> tree_node*, value_range const*, value_range const*) > >> /space/rguenther/src/svn/trunk/gcc/tree-vrp.c:1679 > >> > >> for a PLUS_EXPR of [12254, expon_43] and [-1, -1] yielding > >> (temporarily!) [12254, -1] before supposed to be adjusted by the > >> symbolic bound: > >> > >> /* Adjust the range for possible overflow. */ > >> set_value_range_with_overflow (*vr, expr_type, > >> wmin, wmax, min_ovf, > >max_ovf); > >> ^^^ ICE > >> if (vr->varying_p ()) > >> return; > >> > >> /* Build the symbolic bounds if needed. */ > >> min = vr->min (); > >> max = vr->max (); > >> adjust_symbolic_bound (min, code, expr_type, > >> sym_min_op0, sym_min_op1, > >> neg_min_op0, neg_min_op1); > >> adjust_symbolic_bound (max, code, expr_type, > >> sym_max_op0, sym_max_op1, > >> neg_max_op0, neg_max_op1); > >> type = vr->kind (); > >> > >> I think the refactoring that was applied here is simply not suitable > >> because *vr is _not_ necessarily a valid range before the symbolic > >> bounds have been adjusted. A fix would be sth like the following > >> which I am going to test now. > > > >Sounds reasonable. > > Doesn't work and miscompiles all over the place. > > >Is this PR87640? Because the testcase there is also crashing while > >creating the range right before adjusting the symbolics. > > Might be. > > I'll poke some more tomorrow unless you beat me to it. 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. * gcc.dg/tree-ssa/evrp12.c: New testcase. * gcc.dg/predict-6.c: Adjust. * gcc.dg/tree-ssa/vrp33.c: Disable EVRP. * gcc.dg/tree-ssa/vrp02.c: Likewise. * gcc.dg/tree-ssa/cunroll-9.c: Likewise. Index: gcc/gimple-ssa-evrp-analyze.c =================================================================== --- gcc/gimple-ssa-evrp-analyze.c (revision 265381) +++ gcc/gimple-ssa-evrp-analyze.c (working copy) @@ -203,6 +203,16 @@ evrp_range_analyzer::record_ranges_from_ ordering issues that can lead to worse ranges. */ for (unsigned i = 0; i < vrs.length (); ++i) { + /* But make sure we do not weaken ranges like when + getting first [64, +INF] and then ~[0, 0] from + conditions like (s & 0x3cc0) == 0). */ + value_range *old_vr = get_value_range (vrs[i].first); + value_range tem (old_vr->kind (), old_vr->min (), old_vr->max ()); + tem.intersect (vrs[i].second); + if (tem.kind () == old_vr->kind () + && tem.min () == old_vr->min () + && tem.max () == old_vr->max ()) + continue; push_value_range (vrs[i].first, vrs[i].second); if (is_fallthru && all_uses_feed_or_dominated_by_stmt (vrs[i].first, stmt)) Index: gcc/testsuite/gcc.dg/predict-6.c =================================================================== --- gcc/testsuite/gcc.dg/predict-6.c (revision 265381) +++ gcc/testsuite/gcc.dg/predict-6.c (working copy) @@ -10,9 +10,9 @@ void foo (int base, int bound) int i, ret = 0; for (i = base; i <= bound; i++) { - if (i < base) + if (i <= base) global += bar (i); - if (i < base + 1) + if (i < base + 2) global += bar (i); if (i <= base + 3) global += bar (i); Index: gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c (revision 265381) +++ gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-cunrolli-details" } */ +/* { dg-options "-O2 -fdump-tree-cunrolli-details -fdisable-tree-evrp" } */ void abort (void); int q (void); int a[10]; Index: gcc/testsuite/gcc.dg/tree-ssa/evrp12.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/evrp12.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/evrp12.c (working copy) @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-evrp" } */ + +extern void link_error (); + +void +f3 (unsigned int s) +{ + if ((s & 0x3cc0) == 0) + { + if (s >= -15552U) + link_error (); + } + else + { + if (s <= 0x3f) + link_error (); + } +} + +/* { dg-final { scan-tree-dump-not "link_error" "evrp" } } */ Index: gcc/testsuite/gcc.dg/tree-ssa/vrp02.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/vrp02.c (revision 265381) +++ gcc/testsuite/gcc.dg/tree-ssa/vrp02.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks" } */ +/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks -fdisable-tree-evrp" } */ struct A { Index: gcc/testsuite/gcc.dg/tree-ssa/vrp33.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/vrp33.c (revision 265381) +++ gcc/testsuite/gcc.dg/tree-ssa/vrp33.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-vrp1 -fno-tree-fre" } */ +/* { dg-options "-O2 -fdump-tree-vrp1 -fno-tree-fre -fdisable-tree-evrp" } */ /* This is from PR14052. */ 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); } /* 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); } @@ -2725,16 +2718,6 @@ register_edge_assert_for_2 (tree name, e tmp = build1 (NOP_EXPR, TREE_TYPE (name), tmp); 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, name2); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - add_assert_info (asserts, name2, tmp, comp_code, val); } } @@ -2857,16 +2840,6 @@ register_edge_assert_for_2 (tree name, e cst = fold_build2 (MINUS_EXPR, TREE_TYPE (name2), cst, build_int_cst (TREE_TYPE (name2), 1)); } - - if (dump_file) - { - fprintf (dump_file, "Adding assert for "); - print_generic_expr (dump_file, name2); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - add_assert_info (asserts, name2, tmp, new_comp_code, cst); } } @@ -2931,18 +2904,7 @@ register_edge_assert_for_2 (tree name, e } if (new_val) - { - if (dump_file) - { - fprintf (dump_file, "Adding assert for "); - print_generic_expr (dump_file, name2); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - - add_assert_info (asserts, name2, tmp, new_comp_code, new_val); - } + add_assert_info (asserts, name2, tmp, new_comp_code, new_val); } /* Add asserts for NAME cmp CST and NAME being defined as @@ -3170,16 +3132,6 @@ register_edge_assert_for_2 (tree name, e maxv2 = maxv - minv; } new_val = wide_int_to_tree (type, maxv2); - - if (dump_file) - { - fprintf (dump_file, "Adding assert for "); - print_generic_expr (dump_file, names[i]); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - add_assert_info (asserts, names[i], tmp, LE_EXPR, new_val); } }
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? > /* 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...]
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...] > >
> + if (tem.kind () == old_vr->kind () > + && tem.min () == old_vr->min () > + && tem.max () == old_vr->max ()) > + continue; I think it would be cleaner to use tem.ignore_equivs_equal_p (*old_vr). The goal was to use == when the equivalence bitmap should be taken into account, or ignore_equivs_equal_p() otherwise. (Unless you really really don't want to compare the extremes with vrp_operand_equal_p.) Aldy
On Tue, 23 Oct 2018, Aldy Hernandez wrote: > > > + if (tem.kind () == old_vr->kind () > > + && tem.min () == old_vr->min () > > + && tem.max () == old_vr->max ()) > > + continue; > > I think it would be cleaner to use tem.ignore_equivs_equal_p (*old_vr). The > goal was to use == when the equivalence bitmap should be taken into account, > or ignore_equivs_equal_p() otherwise. Ah, didn't know of that function (and yes, I wanted to ignore equivs). Will try to remember together with the dump thing David noticed. Richard. > (Unless you really really don't want to compare the extremes with > vrp_operand_equal_p.) > > Aldy
On Tue, 23 Oct 2018, Richard Biener wrote: > On Tue, 23 Oct 2018, Aldy Hernandez wrote: > > > > > > + if (tem.kind () == old_vr->kind () > > > + && tem.min () == old_vr->min () > > > + && tem.max () == old_vr->max ()) > > > + continue; > > > > I think it would be cleaner to use tem.ignore_equivs_equal_p (*old_vr). The > > goal was to use == when the equivalence bitmap should be taken into account, > > or ignore_equivs_equal_p() otherwise. > > Ah, didn't know of that function (and yes, I wanted to ignore equivs). > > Will try to remember together with the dump thing David noticed. Like the following. Bootstrapped / tested on x86_64-unknown-linux-gnu, applied. Richard. 2018-10-23 Richard Biener <rguenther@suse.de> * tree-vrp.c (add_assert_info): Guard dump_printf with dump_enabled_p. * gimple-ssa-evrp-analyze.c (evrp_range_analyzer::record_ranges_from_incoming_edge): Use value_range::ignore_equivs_equal_p. Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 265420) +++ gcc/tree-vrp.c (working copy) @@ -2299,9 +2299,10 @@ 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); + if (dump_enabled_p ()) + dump_printf (MSG_NOTE | MSG_PRIORITY_INTERNALS, + "Adding assert for %T from %T %s %T\n", + name, expr, op_symbol_code (comp_code), val); } /* If NAME doesn't have an ASSERT_EXPR registered for asserting Index: gcc/gimple-ssa-evrp-analyze.c =================================================================== --- gcc/gimple-ssa-evrp-analyze.c (revision 265420) +++ gcc/gimple-ssa-evrp-analyze.c (working copy) @@ -209,9 +209,7 @@ evrp_range_analyzer::record_ranges_from_ value_range *old_vr = get_value_range (vrs[i].first); value_range tem (old_vr->kind (), old_vr->min (), old_vr->max ()); tem.intersect (vrs[i].second); - if (tem.kind () == old_vr->kind () - && tem.min () == old_vr->min () - && tem.max () == old_vr->max ()) + if (tem.ignore_equivs_equal_p (*old_vr)) continue; push_value_range (vrs[i].first, vrs[i].second); if (is_fallthru
Thanks! On Tue, Oct 23, 2018, 13:37 Richard Biener <rguenther@suse.de> wrote: > On Tue, 23 Oct 2018, Richard Biener wrote: > > > On Tue, 23 Oct 2018, Aldy Hernandez wrote: > > > > > > > > > + if (tem.kind () == old_vr->kind () > > > > + && tem.min () == old_vr->min () > > > > + && tem.max () == old_vr->max ()) > > > > + continue; > > > > > > I think it would be cleaner to use tem.ignore_equivs_equal_p > (*old_vr). The > > > goal was to use == when the equivalence bitmap should be taken into > account, > > > or ignore_equivs_equal_p() otherwise. > > > > Ah, didn't know of that function (and yes, I wanted to ignore equivs). > > > > Will try to remember together with the dump thing David noticed. > > Like the following. > > Bootstrapped / tested on x86_64-unknown-linux-gnu, applied. > > Richard. > > 2018-10-23 Richard Biener <rguenther@suse.de> > > * tree-vrp.c (add_assert_info): Guard dump_printf with > dump_enabled_p. > * gimple-ssa-evrp-analyze.c > (evrp_range_analyzer::record_ranges_from_incoming_edge): > Use value_range::ignore_equivs_equal_p. > > Index: gcc/tree-vrp.c > =================================================================== > --- gcc/tree-vrp.c (revision 265420) > +++ gcc/tree-vrp.c (working copy) > @@ -2299,9 +2299,10 @@ 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); > + if (dump_enabled_p ()) > + dump_printf (MSG_NOTE | MSG_PRIORITY_INTERNALS, > + "Adding assert for %T from %T %s %T\n", > + name, expr, op_symbol_code (comp_code), val); > } > > /* If NAME doesn't have an ASSERT_EXPR registered for asserting > Index: gcc/gimple-ssa-evrp-analyze.c > =================================================================== > --- gcc/gimple-ssa-evrp-analyze.c (revision 265420) > +++ gcc/gimple-ssa-evrp-analyze.c (working copy) > @@ -209,9 +209,7 @@ evrp_range_analyzer::record_ranges_from_ > value_range *old_vr = get_value_range (vrs[i].first); > value_range tem (old_vr->kind (), old_vr->min (), > old_vr->max ()); > tem.intersect (vrs[i].second); > - if (tem.kind () == old_vr->kind () > - && tem.min () == old_vr->min () > - && tem.max () == old_vr->max ()) > + if (tem.ignore_equivs_equal_p (*old_vr)) > continue; > push_value_range (vrs[i].first, vrs[i].second); > if (is_fallthru >
diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c index e9afa80e191..0748a53cdb8 100644 --- a/gcc/gimple-ssa-evrp-analyze.c +++ b/gcc/gimple-ssa-evrp-analyze.c @@ -206,6 +206,17 @@ evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb) ordering issues that can lead to worse ranges. */ for (unsigned i = 0; i < vrs.length (); ++i) { + /* But make sure we do not weaken ranges like when + getting first [64, +INF] and then ~[0, 0] from + conditions like (s & 0x3cc0) == 0). */ + value_range *old_vr = get_value_range (vrs[i].first); + value_range tem = *old_vr; + tem.equiv = NULL; + vrp_intersect_ranges (&tem, vrs[i].second); + if (tem.type == old_vr->type + && tem.min == old_vr->min + && tem.max == old_vr->max) + continue; push_value_range (vrs[i].first, vrs[i].second); if (is_fallthru && all_uses_feed_or_dominated_by_stmt (vrs[i].first, stmt)) diff --git a/gcc/testsuite/gcc.dg/predict-6.c b/gcc/testsuite/gcc.dg/predict-6.c index 5d6fbf158f2..08ce5cdb81d 100644 --- a/gcc/testsuite/gcc.dg/predict-6.c +++ b/gcc/testsuite/gcc.dg/predict-6.c @@ -10,9 +10,9 @@ void foo (int base, int bound) int i, ret = 0; for (i = base; i <= bound; i++) { - if (i < base) + if (i <= base) global += bar (i); - if (i < base + 1) + if (i < base + 2) global += bar (i); if (i <= base + 3) global += bar (i); diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c b/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c index 0e4407dcbd7..886dc147ad1 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/cunroll-9.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-cunrolli-details" } */ +/* { dg-options "-O2 -fdump-tree-cunrolli-details -fdisable-tree-evrp" } */ void abort (void); int q (void); int a[10]; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/evrp12.c b/gcc/testsuite/gcc.dg/tree-ssa/evrp12.c new file mode 100644 index 00000000000..b3906c23465 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/evrp12.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-evrp" } */ + +extern void link_error (); + +void +f3 (unsigned int s) +{ + if ((s & 0x3cc0) == 0) + { + if (s >= -15552U) + link_error (); + } + else + { + if (s <= 0x3f) + link_error (); + } +} + +/* { dg-final { scan-tree-dump-not "link_error" "evrp" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c index 8d14feadb6a..4be538f5944 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp02.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks" } */ +/* { dg-options "-O2 -fdump-tree-vrp1 -fdelete-null-pointer-checks -fdisable-tree-evrp" } */ struct A { diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c index 75fefa49925..f1d3863943e 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp33.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-vrp1 -fno-tree-fre" } */ +/* { dg-options "-O2 -fdump-tree-vrp1 -fno-tree-fre -fdisable-tree-evrp" } */ /* This is from PR14052. */ diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index cbc2ea2f26b..6f5ec43670e 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -2133,6 +2133,17 @@ add_assert_info (vec<assert_info> &asserts, info.val = val; info.expr = expr; asserts.safe_push (info); + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "Adding assert for "); + print_generic_expr (dump_file, name); + fprintf (dump_file, " from "); + print_generic_expr (dump_file, expr); + fprintf (dump_file, " %s ", op_symbol_code (comp_code)); + print_generic_expr (dump_file, val); + fprintf (dump_file, "\n"); + } } /* If NAME doesn't have an ASSERT_EXPR registered for asserting @@ -2532,16 +2543,6 @@ register_edge_assert_for_2 (tree name, edge 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); } @@ -2559,16 +2560,6 @@ register_edge_assert_for_2 (tree name, edge e, tmp = build1 (NOP_EXPR, TREE_TYPE (name), tmp); 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, name2); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - add_assert_info (asserts, name2, tmp, comp_code, val); } } @@ -2691,16 +2682,6 @@ register_edge_assert_for_2 (tree name, edge e, cst = fold_build2 (MINUS_EXPR, TREE_TYPE (name2), cst, build_int_cst (TREE_TYPE (name2), 1)); } - - if (dump_file) - { - fprintf (dump_file, "Adding assert for "); - print_generic_expr (dump_file, name2); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - add_assert_info (asserts, name2, tmp, new_comp_code, cst); } } @@ -2765,18 +2746,7 @@ register_edge_assert_for_2 (tree name, edge e, } if (new_val) - { - if (dump_file) - { - fprintf (dump_file, "Adding assert for "); - print_generic_expr (dump_file, name2); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - - add_assert_info (asserts, name2, tmp, new_comp_code, new_val); - } + add_assert_info (asserts, name2, tmp, new_comp_code, new_val); } /* Add asserts for NAME cmp CST and NAME being defined as @@ -3004,16 +2974,6 @@ register_edge_assert_for_2 (tree name, edge e, maxv2 = maxv - minv; } new_val = wide_int_to_tree (type, maxv2); - - if (dump_file) - { - fprintf (dump_file, "Adding assert for "); - print_generic_expr (dump_file, names[i]); - fprintf (dump_file, " from "); - print_generic_expr (dump_file, tmp); - fprintf (dump_file, "\n"); - } - add_assert_info (asserts, names[i], tmp, LE_EXPR, new_val); } }