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: abstract wide int binop code from VRP


On Fri, Jul 13, 2018 at 3:18 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Fri, Jul 13, 2018 at 10:05 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> >
> >
> > On 07/13/2018 03:02 AM, Richard Biener wrote:
> > > On Thu, Jul 12, 2018 at 10:12 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > > So besides the general discussion about references/pointers for out parameters
> > > let's stay consistet within related APIs.  This means wide_int_binop should
> > > have a
> > >
> > > wide_int
> > > wide_int_binop (enum tree_code, const wide_int &, const wide_int &,
> > > signop, wi::overflow_type *)
> > >
> > > signature.  Notice how I elided the out wide_int parameter to be the
> > > return value which means
> > > the function isn't supposed to fail which means gcc_unreachable () for
> > > "unhandled" tree codes.
> >
> > wide_int_binop was returning failure for:
> >
> > >     case CEIL_DIV_EXPR:
> > >       if (arg2 == 0)
> > >       return false;
> > >       res = wi::div_ceil (arg1, arg2, sign, &overflow);
> > >       break;
> > >
> > >     case ROUND_DIV_EXPR:
> > >       if (arg2 == 0)
> > >       return false;
> > >       res = wi::div_round (arg1, arg2, sign, &overflow);
> > >       break;
> > etc
> >
> > How do you suggest we indicate success/failure to the caller?
>
> Oh, ok.  Exceptions?  (eh...)
>
> Well, so I guess you can leave the signature as-is apart from turing
> the overflow
> result into a pointer.

Alternatively handle it like wi::sdiv and friends which indicate overflow
and use a zero result.  Thus, remove the "failure" path here.  Of course
when used via the tree interface this probably isn't the desired result
which means this really isn't a general wide-int op with code thing
but only a helper for int_cst_binop :/

So alternatively do the interface I suggested w/o the zero checks
and do the zero checks in the int_const_binop caller.

Richard.

> OK with that change.
> Richard.
>
> > Aldy
> >
> > > It's more like an exceptional state anyway.
> > >
> > > The same goes for the poly_int_binop signature.
> > >
> > > The already existing wi::accumulate_overflow should probably be re-done as
> > >
> > > wi::overflow_type wi::accumulate_overflow (wi::overflow_type,
> > > wi::overflow_type);
> > >
> > > Richard.
> > >
> > >> Thanks for the review!
> > >> Aldy


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