Richard Biener wrote:
>On Thu, Jan 4, 2018 at 10:27 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
>> I don't understand how the check you added helps.
It simply blocks the transformation for infinity:
+ (if (!REAL_VALUE_ISINF (TREE_REAL_CST (@0)))
+ (switch
+ (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+ (op @1 @2))
+ /* For C < 0, use the inverted operator. */
+ (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+ (neg_op @1 @2)))))))
I can't see what is potentially problematic here, it excludes infinity. I can
replace
the flag_unsafe_math_optimizations with HONOR variants and then the infinity
check is no longer required.
>>> 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?
>>
>>
>> That is indeed unclear. HONOR_SIGNED_ZEROS, HONOR_NANS, HONOR_INFINITIES are
>> better defined and could also see some use.
>>
>> 1/X>=0: if X can be zero (and the inverse can be infinite), the result
>> depends on the sign of that zero. If !HONOR_SIGNED_ZEROS ||
>> !HONOR_INFINITIES, it looks like we can simplify to either X>0 or X>=0,
>> doesn't matter which.
Agreed.
>> 1/X>=0 is true iff X is not NaN and the sign bit is not set, so with
>> !HONOR_NANS it could also be turned into a bit test.
That's unlikely more efficient than a compare with zero.
>> It works the same for C/X>=0 with 0<C<infinity. For C=infinity, the main
>> difference is that X=infinity now gives false (if HONOR_NANS).
Yes that's why C=infinity is excluded in the latest version. However if we
already
check HONOR_INFINITIES it wouldn't be needed.
>> C/X>0 is more tricky because small/big may round to zero. For C large enough
>> (assuming denormals, 1 is large enough for instance), the only new issue is
>> X=infinity. For smaller C, we start getting wrong results for finite values
>> of X, and presumably that's where flag_unsafe_math_optimizations comes into
>> play.
Well we can change C/X > 0 to X >= 0. That deals with underflow cases when
X is a normal, and results in the same cases that are wrong as C/X >= 0
(X = -0 and X = +Inf assuming C>0), so !HONOR_SIGNED_ZEROS &&
!HONOR_INFINITIES should be sufficient for all cases.
>> If we consider flag_unsafe_math_optimizations as a kitchen sink that implies
>> !HONOR_SIGNED_ZEROS && !HONOR_INFINITIES, then you don't even need your
>> REAL_VALUE_ISINF test.
Indeed. Changing it to use more descriptive tests is a good idea indeed as it
makes it
more explicit which cases are not supported.
> Without looking at the latest patch -funsafe-math-optimizations is a
> kitchen-sink
> we should avoid at all cost. In the past we somewhat decided to break it up
> properly (-fassociative-math and friends have been added for example),
> and we really
> don't want to add more stuff under this umbrella (there's too much
> under it already).
> I'm not sure if C / x op 0.0 to x op 0.0 is association though (I'm
> quite sure it isn't),
> so citing association examples doesn't work for me here ;)
Marc's proposal to use HONOR_* instead of flag_unsafe_math_optimizations
should work, I'll give it a go. Maybe in the future we should aim to replace all
existing uses.
> It might be we need some new flag like -fassume-infinite-precision-math to
> cover the underflow issue? We do have -ffinite-math-only but that
> shouldn't allow
> GCC to possibly introduce +-Inf for overflow - GCC just can assume there are
> no overflows in the original user expression.
We can avoid the underflow for normal numbers, so I don't think there is need
for a new flag.
Wilco
