Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez <aldyh@redhat.com>]

On 3/10/23 11:29, Jakub Jelinek wrote:
> On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote:
>> Meh - I wonder if we can avoid all this by making float_widen_lhs_range
>> friend of frange and simply access m_min/m_max directly and use the
>> copy-CTOR to copy bounds and nan state ... after all verify_range
>> will likely fail after you restore flag_finite_math_only ...
> 
> I'll defer such changes to Aldy.
> 
> As for verification, I think verify_range will not fail on it, it mainly
> checks whether it is normalized (e.g. if minimum is frange_val_min and
> maximum is frange_val_max and NaNs are possible with both signs (if NaNs
> are supported) then it is VR_VARYING etc.).  It doesn't check if the actual
> VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is
> just equality check.
> Of course, behavior of wider than varying ranges is still unexpected in
> many ways, say the union_ of such a range and VR_VARYING will ICE etc.
> 
> Now, I guess another possibility for the reverse ops over these wider ranges
> would be avoid calling fold_range in the reverse ops, but call rv_fold
> directly or have fold_range variant which would instead of the op1, op2
> argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it
> would use those const REAL_VALUE_TYPE &op??b in preference to
> op?.{lower,upper}_bound () or perhaps normal fold_range be implemented
> in terms of this extended fold_range.  Then we wouldn't need to bother with
> these non-standard franges...
> 
>> But OK for the moment.
> 
> Thanks, committed.

I'm not a big fan of constructing ranges that break all our internal 
consistency checks.  I'd also prefer to add another constructor (or add 
a flag to the current constructor) instead of making range-op-float 
routines friends.  We also have bits in the vrange (or frange) that we 
could use for other semantics, especially since frange_storage can be 
streamlined when stored in GC/etc.

I'm on PTO this week.  Could we revisit this next week?  And if worse 
comes to worse, leave it as is, and fix it properly next release?

Thanks for your work on this.
Aldy