On Fri, Jul 13, 2018 at 3:18 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On Fri, Jul 13, 2018 at 10:05 AM Aldy Hernandez <al...@redhat.com> wrote: > > > > > > > > On 07/13/2018 03:02 AM, Richard Biener wrote: > > > On Thu, Jul 12, 2018 at 10:12 AM Aldy Hernandez <al...@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
