Revert "Revert "LangRef: Clarify llvm.minnum and llvm.maxnum about sNaN and signed zero (#112852)""

[arsenm]

Reverts #168838

Justification is confused and this did not receive adequate discussion, particularly during a holiday week

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-llvm-ir

Author: Matt Arsenault (arsenm)

Changes

Reverts llvm/llvm-project#168838

Justification is confused and this did not receive adequate discussion, particularly during a holiday week

---

Full diff: https://github.com/llvm/llvm-project/pull/170067.diff

2 Files Affected:

• (modified) llvm/docs/LangRef.rst (+56-54)
• (modified) llvm/include/llvm/CodeGen/ISDOpcodes.h (+15-5)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index a57351f9598e2..02865f8a29c67 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -17298,9 +17298,8 @@ LLVM Implementation:
 """"""""""""""""""""
 
 LLVM implements all ISO C flavors as listed in this table, except in the
-default floating-point environment exceptions are ignored and return value
-is non-deterministic if one or both inputs are sNaN. The constrained
-versions of the intrinsics respect the exception behavior and sNaN.
+default floating-point environment exceptions are ignored. The constrained
+versions of the intrinsics respect the exception behavior.
 
 .. list-table::
    :header-rows: 1
@@ -17332,7 +17331,7 @@ versions of the intrinsics respect the exception behavior and sNaN.
      - qNaN, invalid exception
 
    * - ``+0.0 vs -0.0``
-     - either one
+     - +0.0(max)/-0.0(min)
      - +0.0(max)/-0.0(min)
      - +0.0(max)/-0.0(min)
 
@@ -17376,22 +17375,30 @@ type.
 
 Semantics:
 """"""""""
+Follows the semantics of minNum in IEEE-754-2008, except that -0.0 < +0.0 for the purposes
+of this intrinsic. As for signaling NaNs, per the minNum semantics, if either operand is sNaN,
+the result is qNaN. This matches the recommended behavior for the libm
+function ``fmin``, although not all implementations have implemented these recommended behaviors.
+
+If either operand is a qNaN, returns the other non-NaN operand. Returns NaN only if both operands are
+NaN or if either operand is sNaN. Note that arithmetic on an sNaN doesn't consistently produce a qNaN,
+so arithmetic feeding into a minnum can produce inconsistent results. For example,
+``minnum(fadd(sNaN, -0.0), 1.0)`` can produce qNaN or 1.0 depending on whether ``fadd`` is folded.
 
-Follows the IEEE-754-2008 semantics for minNum, except for handling of
-signaling NaNs. This matches the behavior of libm's fmin.
+IEEE-754-2008 defines minNum, and it was removed in IEEE-754-2019. As the replacement, IEEE-754-2019
+defines :ref:`minimumNumber <i_minimumnum>`.
 
-If either operand is a NaN, returns the other non-NaN operand. Returns
-NaN only if both operands are NaN. If the operands compare equal,
-returns either one of the operands. For example, this means that
-fmin(+0.0, -0.0) non-deterministically returns either operand (-0.0
-or 0.0).
+If the intrinsic is marked with the nsz attribute, then the effect is as in the definition in C
+and IEEE-754-2008: the result of ``minnum(-0.0, +0.0)`` may be either -0.0 or +0.0.
 
-Unlike the IEEE-754-2008 behavior, this does not distinguish between
-signaling and quiet NaN inputs. If a target's implementation follows
-the standard and returns a quiet NaN if either input is a signaling
-NaN, the intrinsic lowering is responsible for quieting the inputs to
-correctly return the non-NaN input (e.g. by using the equivalent of
-``llvm.canonicalize``).
+Some architectures, such as ARMv8 (FMINNM), LoongArch (fmin), MIPSr6 (min.fmt), PowerPC/VSX (xsmindp),
+have instructions that match these semantics exactly; thus it is quite simple for these architectures.
+Some architectures have similar ones while they are not exact equivalent. Such as x86 implements ``MINPS``,
+which implements the semantics of C code ``a<b?a:b``: NUM vs qNaN always return qNaN. ``MINPS`` can be used
+if ``nsz`` and ``nnan`` are given.
+
+For existing libc implementations, the behaviors of fmin may be quite different on sNaN and signed zero behaviors,
+even in the same release of a single libm implementation.
 
 .. _i_maxnum:
 
@@ -17428,21 +17435,30 @@ type.
 
 Semantics:
 """"""""""
-Follows the IEEE-754-2008 semantics for maxNum except for the handling of
-signaling NaNs. This matches the behavior of libm's fmax.
+Follows the semantics of maxNum in IEEE-754-2008, except that -0.0 < +0.0 for the purposes
+of this intrinsic. As for signaling NaNs, per the maxNum semantics, if either operand is sNaN,
+the result is qNaN. This matches the recommended behavior for the libm
+function ``fmax``, although not all implementations have implemented these recommended behaviors.
+
+If either operand is a qNaN, returns the other non-NaN operand. Returns NaN only if both operands are
+NaN or if either operand is sNaN. Note that arithmetic on an sNaN doesn't consistently produce a qNaN,
+so arithmetic feeding into a maxnum can produce inconsistent results. For example,
+``maxnum(fadd(sNaN, -0.0), 1.0)`` can produce qNaN or 1.0 depending on whether ``fadd`` is folded.
 
-If either operand is a NaN, returns the other non-NaN operand. Returns
-NaN only if both operands are NaN. If the operands compare equal,
-returns either one of the operands. For example, this means that
-fmax(+0.0, -0.0) non-deterministically returns either operand (-0.0
-or 0.0).
+IEEE-754-2008 defines maxNum, and it was removed in IEEE-754-2019. As the replacement, IEEE-754-2019
+defines :ref:`maximumNumber <i_maximumnum>`.
 
-Unlike the IEEE-754-2008 behavior, this does not distinguish between
-signaling and quiet NaN inputs. If a target's implementation follows
-the standard and returns a quiet NaN if either input is a signaling
-NaN, the intrinsic lowering is responsible for quieting the inputs to
-correctly return the non-NaN input (e.g. by using the equivalent of
-``llvm.canonicalize``).
+If the intrinsic is marked with the nsz attribute, then the effect is as in the definition in C
+and IEEE-754-2008: the result of maxnum(-0.0, +0.0) may be either -0.0 or +0.0.
+
+Some architectures, such as ARMv8 (FMAXNM), LoongArch (fmax), MIPSr6 (max.fmt), PowerPC/VSX (xsmaxdp),
+have instructions that match these semantics exactly; thus it is quite simple for these architectures.
+Some architectures have similar ones while they are not exact equivalent. Such as x86 implements ``MAXPS``,
+which implements the semantics of C code ``a>b?a:b``: NUM vs qNaN always return qNaN. ``MAXPS`` can be used
+if ``nsz`` and ``nnan`` are given.
+
+For existing libc implementations, the behaviors of fmin may be quite different on sNaN and signed zero behaviors,
+even in the same release of a single libm implementation.
 
 .. _i_minimum:
 
@@ -20326,12 +20342,8 @@ The '``llvm.vector.reduce.fmax.*``' intrinsics do a floating-point
 matches the element-type of the vector input.
 
 This instruction has the same comparison semantics as the '``llvm.maxnum.*``'
-intrinsic. That is, the result will always be a number unless all elements of
-the vector are NaN. For a vector with maximum element magnitude 0.0 and
-containing both +0.0 and -0.0 elements, the sign of the result is unspecified.
-
-If the intrinsic call has the ``nnan`` fast-math flag, then the operation can
-assume that NaNs are not present in the input vector.
+intrinsic.  If the intrinsic call has the ``nnan`` fast-math flag, then the
+operation can assume that NaNs are not present in the input vector.
 
 Arguments:
 """"""""""
@@ -20359,12 +20371,8 @@ The '``llvm.vector.reduce.fmin.*``' intrinsics do a floating-point
 matches the element-type of the vector input.
 
 This instruction has the same comparison semantics as the '``llvm.minnum.*``'
-intrinsic. That is, the result will always be a number unless all elements of
-the vector are NaN. For a vector with minimum element magnitude 0.0 and
-containing both +0.0 and -0.0 elements, the sign of the result is unspecified.
-
-If the intrinsic call has the ``nnan`` fast-math flag, then the operation can
-assume that NaNs are not present in the input vector.
+intrinsic. If the intrinsic call has the ``nnan`` fast-math flag, then the
+operation can assume that NaNs are not present in the input vector.
 
 Arguments:
 """"""""""
@@ -22751,7 +22759,7 @@ This is an overloaded intrinsic.
 Overview:
 """""""""
 
-Predicated floating-point IEEE-754 minNum of two vectors of floating-point values.
+Predicated floating-point IEEE-754-2008 minNum of two vectors of floating-point values.
 
 
 Arguments:
@@ -22800,7 +22808,7 @@ This is an overloaded intrinsic.
 Overview:
 """""""""
 
-Predicated floating-point IEEE-754 maxNum of two vectors of floating-point values.
+Predicated floating-point IEEE-754-2008 maxNum of two vectors of floating-point values.
 
 
 Arguments:
@@ -24099,10 +24107,7 @@ result type. If only ``nnan`` is set then the neutral value is ``-Infinity``.
 
 This instruction has the same comparison semantics as the
 :ref:`llvm.vector.reduce.fmax <int_vector_reduce_fmax>` intrinsic (and thus the
-'``llvm.maxnum.*``' intrinsic). That is, the result will always be a number
-unless all elements of the vector and the starting value are ``NaN``. For a
-vector with maximum element magnitude ``0.0`` and containing both ``+0.0`` and
-``-0.0`` elements, the sign of the result is unspecified.
+'``llvm.maxnum.*``' intrinsic).
 
 To ignore the start value, the neutral value can be used.
 
@@ -24169,10 +24174,7 @@ result type. If only ``nnan`` is set then the neutral value is ``+Infinity``.
 
 This instruction has the same comparison semantics as the
 :ref:`llvm.vector.reduce.fmin <int_vector_reduce_fmin>` intrinsic (and thus the
-'``llvm.minnum.*``' intrinsic). That is, the result will always be a number
-unless all elements of the vector and the starting value are ``NaN``. For a
-vector with maximum element magnitude ``0.0`` and containing both ``+0.0`` and
-``-0.0`` elements, the sign of the result is unspecified.
+'``llvm.minnum.*``' intrinsic).
 
 To ignore the start value, the neutral value can be used.
 
@@ -29044,7 +29046,7 @@ The third argument specifies the exception behavior as described above.
 Semantics:
 """"""""""
 
-This function follows the IEEE-754 semantics for maxNum.
+This function follows the IEEE-754-2008 semantics for maxNum.
 
 
 '``llvm.experimental.constrained.minnum``' Intrinsic
@@ -29076,7 +29078,7 @@ The third argument specifies the exception behavior as described above.
 Semantics:
 """"""""""
 
-This function follows the IEEE-754 semantics for minNum.
+This function follows the IEEE-754-2008 semantics for minNum.
 
 
 '``llvm.experimental.constrained.maximum``' Intrinsic
diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h
index a9fdf803a5511..b32f3dacbb3a4 100644
--- a/llvm/include/llvm/CodeGen/ISDOpcodes.h
+++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h
@@ -1048,13 +1048,20 @@ enum NodeType {
   LRINT,
   LLRINT,
 
-  /// FMINNUM/FMAXNUM - Perform floating-point minimum or maximum on two
-  /// values.
+  /// FMINNUM/FMAXNUM - Perform floating-point minimum maximum on two values,
+  /// following IEEE-754 definitions except for signed zero behavior.
   ///
-  /// In the case where a single input is a NaN (either signaling or quiet),
-  /// the non-NaN input is returned.
+  /// If one input is a signaling NaN, returns a quiet NaN. This matches
+  /// IEEE-754 2008's minNum/maxNum behavior for signaling NaNs (which differs
+  /// from 2019).
   ///
-  /// The return value of (FMINNUM 0.0, -0.0) could be either 0.0 or -0.0.
+  /// These treat -0 as ordered less than +0, matching the behavior of IEEE-754
+  /// 2019's minimumNumber/maximumNumber.
+  ///
+  /// Note that that arithmetic on an sNaN doesn't consistently produce a qNaN,
+  /// so arithmetic feeding into a minnum/maxnum can produce inconsistent
+  /// results. FMAXIMUN/FMINIMUM or FMAXIMUMNUM/FMINIMUMNUM may be better choice
+  /// for non-distinction of sNaN/qNaN handling.
   FMINNUM,
   FMAXNUM,
 
@@ -1068,6 +1075,9 @@ enum NodeType {
   ///
   /// These treat -0 as ordered less than +0, matching the behavior of IEEE-754
   /// 2019's minimumNumber/maximumNumber.
+  ///
+  /// Deprecated, and will be removed soon, as FMINNUM/FMAXNUM have the same
+  /// semantics now.
   FMINNUM_IEEE,
   FMAXNUM_IEEE,
 

[phoebewang]

@arsenm I'm confusing here. If you believe we can revert a patch arbitrarily as long as we don't agree with it. You shouldn't block a simple revert like #138451 and revert a revert patch. The holiday reason also surprises me. I'd not be surprised if anyone revert it use it. But you are in the reviewer list at the beginning when the patch created on 20th and you were active till 27th according to GH record. So my understanding was you don't object it at least.

[arsenm]

You shouldn't block a simple revert like #138451

This is not a simple revert

But you are in the reviewer list at the beginning when the patch created on 20th and you were active till 27th according to GH record. So my understanding was you don't object it at least.

That's not how anything works.

[RalfJung]

This revert re-introduces the bug that the x86 backend is wrong. @arsenm what do you propose a frontend should do if it wants IEEE754-2008 minNum (not IEEE754-2019 minimumNumber) semantics? For many years, that's what llvm.minnum did, and so that's what frontends like Rust want. IOW, this docs change is a breaking change. The 2008 semantics are also what many LLVM backends and optimizations do when they see llvm.minnum. It's not enough to just change the docs, you need to change optimizations and backends as well -- and from all the discussion that has been had, it's not clear that there is consensus to do that. Furthermore, people have waited many months since the first doc change, and there has apparently been zero progress towards actually implementing the new semantics in backends and optimizations.