Re: [PATCH] Fix some EVRP stupidness
Thanks! On Tue, Oct 23, 2018, 13:37 Richard Biener 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 > > * 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 >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 >
Re: [PATCH] Fix some EVRP stupidness
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 * 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 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
Re: [PATCH] Fix some EVRP stupidness
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
Re: [PATCH] Fix some EVRP stupidness
+ 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
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
> >
> > * 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
> >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
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [PATCH] Fix some EVRP stupidness
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
>
> * 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
>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...]
Re: [PATCH] Fix some EVRP stupidness
On Thu, 18 Oct 2018, Richard Biener wrote:
> On October 18, 2018 5:42:56 PM GMT+02:00, Aldy Hernandez
> 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
* 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 ());
+
Re: [PATCH] Fix some EVRP stupidness
On October 18, 2018 5:42:56 PM GMT+02:00, Aldy Hernandez 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
Re: [PATCH] Fix some EVRP stupidness
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
Re: [PATCH] Fix some EVRP stupidness
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::overflow_type ,
underflow. +1 indicates overflow. 0 indicates neither. */
static void
-set_value_range_with_overflow (value_range ,
+set_value_range_with_overflow (value_range_kind , tree , tree
,
tree type,
const wide_int , const wide_int ,
wi::overflow_type min_ovf,
@@ -1341,7 +1341,7 @@ set_value_range_with_overflow (value_range ,
range covers all values. */
if (prec == 1 && wi::lt_p (wmax, wmin, sgn))
{
- set_value_range_to_varying ();
+ kind = VR_VARYING;
return;
}
@@ -1357,13 +1357,15 @@ set_value_range_with_overflow (value_range ,
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 ,
types values. */
if (covers || wi::cmp (tmin, tmax, sgn) > 0)
{
- set_value_range_to_varying ();
+ 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;
+
