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/13/23 08:50, Richard Biener wrote:
> On Mon, 13 Mar 2023, Aldy Hernandez wrote:
> 
>>
>>
>> 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?
> 
> Yes, sure - I just noticed that we're forced to use high-level API for
> something that's quite low-level and should be internal (a range
> "breaking" internal consistency checks).

Yeah, let's fix the API.  No sense hacking around things if what we need 
is to tweak the design.

I don't like hacking around things.  It always comes back to bite me ;-).

Aldy