On Mon, Sep 19, 2022 at 3:42 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On Mon, Sep 19, 2022 at 2:52 PM Aldy Hernandez <al...@redhat.com> wrote: > > > > On Mon, Sep 19, 2022 at 10:14 AM Richard Biener > > <richard.guent...@gmail.com> wrote: > > > > > > On Mon, Sep 19, 2022 at 9:59 AM Aldy Hernandez <al...@redhat.com> wrote: > > > > > > > > ISTM that a specifically nonnegative range should not contain -NAN, > > > > otherwise signbit_p() would return false, because we'd be unsure of the > > > > sign. > > > > > > > > Do y'all agree? > > > > > > what tree_expr_nonnegative_p actually means isn't 100% clear. For > > > REAL_CST > > > it actually looks at the sign-bit but we have > > > > > > (simplify > > > /* copysign(x,y) -> fabs(x) if y is nonnegative. */ > > > (COPYSIGN_ALL @0 tree_expr_nonnegative_p@1) > > > (abs @0)) > > > > > > is abs (@0) OK for sNaNs and -NaN/+NaN? > > > > At least for real_value's, ABS_EXPR works on NAN's. There's no > > special code dealing with them. We just clear the sign bit: > > > > real_arithmetic: > > case ABS_EXPR: > > *r = *op0; > > r->sign = 0; > > break; > > > > > > > > And we have > > > > > > /* Convert abs[u] (X) where X is nonnegative -> (X). */ > > > (simplify > > > (abs tree_expr_nonnegative_p@0) > > > @0) > > > > > > where at least sNaN -> qNaN would be dropped? > > > > > > And of course > > > > > > (simplify > > > /* signbit(x) -> 0 if x is nonnegative. */ > > > (SIGNBIT tree_expr_nonnegative_p@0) > > > { integer_zero_node; }) > > > > > > that is, is tree_expr_nonnegative_p actually tree_expr_sign or > > > does tree_expr_nonnegative (x) mean x >= (typeof(X)) 0 > > > or !(x < (typeof(X))0)? > > > > I have no idea, but I'm happy to have frange::set_nonnegative() do > > whatever you agree on. > > > > Actually TBH, ranger only uses set_nonnegative for call's, not much else: > > > > if (range_of_builtin_call (r, call, src)) > > ; > > else if (gimple_stmt_nonnegative_warnv_p (call, &strict_overflow_p)) > > r.set_nonnegative (type); > > else if (gimple_call_nonnull_result_p (call) > > || gimple_call_nonnull_arg (call)) > > r.set_nonzero (type); > > else > > r.set_varying (type); > > > > but I guess it's good we do the right thing for correctness sake, and > > if it ever gets used by someone else. > > > > > > > > That said, 'set_nonnegative' could be interpreted to be without > > > NaNs? > > > > Sounds good to me. How's this? > > Hmm, I merely had lots of questions above so I think to answer them > we at least should document what 'set_nonnegative' implies in the > abstract vrange class. > > It's probably safer to keep NaN in. For example unfolded copysign (x, 1.) > will return true for x == -NaN but the result will be a NaN.
Come to think of it, perhaps having set_nonnegative in the API is overkill. It's either nonsensical or awkward for say pointers or strings. There is only one use of it in all of ranger. It's only used to set a range for the result from gimple_stmt_nonnegative* on an unknown call: else if (gimple_stmt_nonnegative_warnv_p (call, &strict_overflow_p)) r.set_nonnegative (type); I would just remove the API entry point and push whatever we decide onto the actual use. No point confusing everyone at large. So... what does gimple_stmt_nonnegative_warnv_p imply for floats? My gut feeling is [+0.0,+INF] U +NAN, but I'm happy to do whatever y'all agree on. Aldy