[PATCH] D136097: InstSimplify: Fold fdiv nnan nsz x, 0 -> inf
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 17 13:52:14 PDT 2022
spatel added a comment.
In D136097#3863158 <https://reviews.llvm.org/D136097#3863158>, @nlopes wrote:
> In D136097#3862946 <https://reviews.llvm.org/D136097#3862946>, @spatel wrote:
>
>> In D136097#3862928 <https://reviews.llvm.org/D136097#3862928>, @kpn wrote:
>>
>>> It looks like by 754 sec 7.3 a (-x)/(+0) == -Inf, but the nsz is documented as saying the sign of the result is "insignificant", so I think we're good here.
>>>
>>> LGTM
>>
>> "nsz" means the sign of a **zero** result is insignificant, not any result. -42.0 / 0.0 should be -Inf, not +Inf?
>
> It's documented as ignoring the sign of a zero argument or a zero result.
> Thus, when nsw is involved, we should consider the result of -42.0 / 0.0 and -42.0 / -0.0. This case the 0.0 is a constant, but that doesn't matter.
>
> Alive2 is correct here AFAICT.
Ah, thanks for clearing that up. The "nsz" wording has always been a less solid than I'd like, but I'm not sure how to change it.
I suspect this transform would cause some fallout with users because it's a stretch to go from -0.0 isn't different than 0.0, therefore, -Inf isn't different than +Inf. There was a suggestion in a previous discussion that we really need entirely new FP types to use with fast-math-flags, so it's clear that -0.0, Inf, or NaN values simply do not exist when the corresponding compile flag is used, but I don't think anyone has looked seriously at implementing that. There's more discussion about the general problem here:
https://github.com/llvm/llvm-project/issues/51601
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136097/new/
https://reviews.llvm.org/D136097
More information about the llvm-commits
mailing list