On Wed, Jul 18, 2018 at 2:05 PM Aldy Hernandez <al...@redhat.com> wrote:
>
> Hi again!
>
> Well, since this hasn't been reviewed and I'm about to overhaul the
> TYPE_OVERFLOW_WRAPS code anyhow, might as well lump it all in one patch.
>
> On 07/16/2018 09:19 AM, Aldy Hernandez wrote:
> > Howdy!
> >
> > I've abstracted out the cross product calculations into its own
> > function, and have adapted it to deal with wide ints so it's more
> > reusable. It required some shuffling around, and implementing things a
> > bit different, but things should be behave as before.
> >
> > I also renamed vrp_int_const_binop to make its intent clearer,
> > especially now that it's really just a wrapper to wide_int_binop that
> > deals with overflow.
> >
> > (If wide_int_binop_overflow is generally useful, perhaps we could merge
> > it with wide_int_overflow.)
>
> This is the same as the previous patch, plus I'm abstracting the
> TYPE_OVERFLOW_WRAPS code as well. With this, the code dealing with
> MULT_EXPR in vrp gets reduced to handling value_range specific stuff.
> Yay code re-use!
>
> A few notes:
>
> This is dead code. I've removed it:
>
> - /* If we have an unsigned MULT_EXPR with two VR_ANTI_RANGEs,
> - drop to VR_VARYING. It would take more effort to compute a
> - precise range for such a case. For example, if we have
> - op0 == 65536 and op1 == 65536 with their ranges both being
> - ~[0,0] on a 32-bit machine, we would have op0 * op1 == 0, so
> - we cannot claim that the product is in ~[0,0]. Note that we
> - are guaranteed to have vr0.type == vr1.type at this
> - point. */
> - if (vr0.type == VR_ANTI_RANGE
> - && !TYPE_OVERFLOW_UNDEFINED (expr_type))
> - {
> - set_value_range_to_varying (vr);
> - return;
> - }
>
> Also, the vrp_int typedef has a weird name, especially when we have
> widest2_int in gimple-fold.c that does the exact thing. I've moved the
> common code to wide-int.h and tree.h so we can all share :).
>
> At some point we could move the wide_int_range* and wide_int_binop* code
> into its own file.
Yes.
> Tested on x86-64 Linux.
>
> OK?
+bool
+wide_int_range_cross_product_wrapping (wide_int &res_lb,
+ wide_int &res_ub,
please rename this to sth like wide_int_range_mult_wrapping
because it only handles multiplication to not confuse it with
the other function.
Otherwise OK (and sorry for the delay in reviewing).
Thanks,
Richard.
