This is an archive of the discontinued LLVM Phabricator instance.

ValueTracking: Implement computeKnownFPClass for minnum/maxnum
ClosedPublic

Authored by arsenm on Apr 8 2023, 4:33 PM.

Diff Detail

Event Timeline

arsenm created this revision.Apr 8 2023, 4:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2023, 4:33 PM
arsenm requested review of this revision.Apr 8 2023, 4:33 PM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: wdng. · View Herald Transcript
arsenm updated this revision to Diff 512155.Apr 10 2023, 8:24 AM

Fix optimistic nan handling if first operand is known non-nan but the second isn't

minnum(0.0, 0.0) can return -0.0, so the handling around positive/negative zero isn't correct. Also DAZ/FTZ mode means the subnormal/zero return bits are going to be off.

It's possible to get cleverer about bounds on the minimum and maximum values (e.g., all negative values on one operand and all positive values on the other, we know it must be all positive values), but I don't think that belongs as part of this analysis.

minnum(0.0, 0.0) can return -0.0, so the handling around positive/negative zero isn't correct.

How is it not correct? If either operand can be +/- 0 such is reported

minnum(0.0, 0.0) can return -0.0, so the handling around positive/negative zero isn't correct.

How is it not correct? If either operand can be +/- 0 such is reported

If both inputs have KnownFPClass of positive zero but not negative zero, the result of the |= won't set the bit appropriately.

minnum(0.0, 0.0) can return -0.0, so the handling around positive/negative zero isn't correct.

The sign is of zero is not indeterminate. minnum(0.0, 0.0) cannot choose to introduce a signed zero. The ordering of a positive zero and negative zero is undetermined.

arsenm updated this revision to Diff 514012.Apr 16 2023, 7:58 AM

Fix denormal handling

minnum(0.0, 0.0) can return -0.0, so the handling around positive/negative zero isn't correct.

The sign is of zero is not indeterminate. minnum(0.0, 0.0) cannot choose to introduce a signed zero. The ordering of a positive zero and negative zero is undetermined.

Section 11 states "Do not depend on the sign of a zero result or the quantum of a decimal result for minNum(x, y), maxNum(x, y), minNumMag(x, y), or maxNumMag(x, y) when x and y are equal.". But the definition for minNum and maxNum state it returns x or y. Returning -0 would be returning a value that is neither x nor y, so I maintain the handling here is correct as-is

The semantics in the LangRef say:

If the operands compare equal, returns a value that compares equal to both operands. This means that fmin(+/-0.0, +/-0.0) could return either -0.0 or 0.0.

This is distinctly weaker semantics than IEEE 754-2008's minNum. C2x depends on IEEE 754-2019 (which drops minNum), but interestingly, neither C11 nor C17 actually try to relate fmin to IEEE 754 minNum. minNum is clearly required to return one of its input arguments.

C is somewhat unclear on the subject, but C2x does mention that fmin differs from fminimum "in which signed zero is returned when the arguments are differently signed zeros", which would suggest that fmin(-0, -0) is -0 and fmin(+0, +0) is +0. In other words, the most reasonable interpretation of C is that fmin (like minNum) returns one of its arguments.

I suspect the wording in the LangRef doesn't match the actual intended semantics. If the operands compare equal, it should return "either value" instead of "a value that compares equal to both operands".

I suspect the wording in the LangRef doesn't match the actual intended semantics. If the operands compare equal, it should return "either value" instead of "a value that compares equal to both operands".

Fixed by D148792

jcranmer-intel accepted this revision.May 2 2023, 9:07 AM
This revision is now accepted and ready to land.May 2 2023, 9:07 AM