On 11/15/2017 08:36 AM, Wilco Dijkstra wrote:
> Richard Biener wrote:
>> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <wilco.dijks...@arm.com>
>> wrote:
>
>>> +(if (flag_unsafe_math_optimizations)
>>> + /* Simplify (C / x op 0.0) to x op 0.0 for C > 0. */
>>> + (for op (lt le gt ge)
>>> + neg_op (gt ge lt le)
>>> + (simplify
>>> + (op (rdiv REAL_CST@0 @1) real_zerop@2)
>>> + (switch
>>> + (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>>
>> Note that real_less (0., +Inf) so I think you either need to check C is
>> 'normal'
>> or ! HONOR_INFINITIES.
>
> Yes, it was missing an explicit check for infinity, now added.
>
>> There's also the underflow issue I guess this is what
>> -funsafe-math-optimizations
>> is for. I think ignoring underflows is dangerous though.
>
> We could change C / x > 0 to x >= 0 so the underflow case is included.
> However that still means x == 0.0 would behave differently - so the question
> is
> what exactly does -funsafe-math-optimization allow?
Well, we largely define what it means. I believe we have in the past
stated that it can't break SPEC. It's as good a rule as anything,
though it is underspecified (what version, what other flags, what
targets, etc).
With that bit of background, I believe that -funsafe-math-optimizations
has been allowed to inhibit or introduce underflows (reassociation in
particular I think does this and is enabled by
-funsafe-math-optimizations). So I don't think ignoring underflow
should inherently block this patch.
>
>
>>> + (for cmp (lt le gt ge)
>>> + neg_cmp (gt ge lt le)
>>> + /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0. */
>>> + (simplify
>>> + (cmp (mult @0 REAL_CST@1) REAL_CST@2)
>>> + (with
>>> + { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
>>> + (if (tem)
>>> + (switch
>>> + (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
>>> + (cmp @0 { tem; }))
>>> + (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
>>> + (neg_cmp @0 { tem; })))))))
>>
>>
>> Drops possible overflow/underflow in x * C1 and may create underflow
>> or overflow with C2/C1 which you should detect here at least.
>
> I've added checks for this, however I thought -funsafe-math-optimizations is
> allowed to insert/remove underflow/overflow, like in these cases:
>
> (x * 1e20f) * 1e20f and (x * 1e40f) * 1e-30f.
Right. That's my understanding as well.
>
>> Existing overflows may be guarded against with a HONOR_INFINITIES check.
>
> Not sure what you mean with this?
>
>> When overflow/underflow can be disregarded is there any reason remaining to
>> make this guarded by flag_unsafe_math_optimizations? Are there any cases
>> where rounding issues can flip the comparison result?
>
> I think it needs to remain under -funsafe-math-optimizations. Here is the
> updated
> version:
Richi has taken the lead on this review and should probably own it.
Just thought it was worth chiming in on the underflow issue.
jeff
