This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |