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]

Re: cleanup cross product code in VRP


On Wed, Jul 18, 2018 at 2:05 PM Aldy Hernandez <aldyh@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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]