On 11/12/18 7:12 AM, Richard Biener wrote:
This mainly tries to rectify the workaround I put in place for ipa-cp.c
needing to build value_range instead of value_range_base for calling
extract_range_from_unary_expr.
To make this easier I moved more set_* functions to methods.
Then for some reason I chose to fix the rathole of equiv bitmap sharing
after finding at least one real bug
By the way, I've seen that the equiv_add() calls in vr-values.c
sometimes set equivalences for VARYING and UNDEFINED which in theory
shouldn't happen. I've been too chicken to follow that hole.
I think we should assert everywhere that we set the equivalences, that
we're not talking about VARYING or UNDEFINED.
@@ -6168,37 +6172,30 @@ value_range::union_helper (value_range *vr0, const
value_range *vr1)
return;
}
- value_range saved (*vr0);
value_range_kind vr0type = vr0->kind ();
tree vr0min = vr0->min ();
tree vr0max = vr0->max ();
union_ranges (&vr0type, &vr0min, &vr0max,
vr1->kind (), vr1->min (), vr1->max ());
- *vr0 = value_range (vr0type, vr0min, vr0max);
- if (vr0->varying_p ())
+ /* Work on a temporary so we can still use vr0 when union returns varying.
*/
+ value_range tem;
+ tem.set_and_canonicalize (vr0type, vr0min, vr0max);
+ if (tem.varying_p ())
I'm not a big fan of the code duplication in the union chunks. You're
adding more places that need to be kept in sync.
I think value_range::union_ could be easily coded as:
value_range_base::union_ (other);
union_helper (this, other);
if (flag_checking)
check ();
And have union_helper only deal with the equivalence stuff. Call it
union_equivs? You'd have to clear the equivalences if the range just
became a varying/undefined, as both of those should in theory never have
equivalences.
Also, is there a reason why you implemented value_range_base::union_ but
not the corresponding for intersect? I would guess it'd be needed
sooner or later.
Thanks for working on this.
Aldy
