CodeGen: Strengthen definition of F{MIN|MAX}NUM_IEEE nodes #85195
Conversation
Previously these were declared as having the 2008 behavior, with underspecified signed zero handling. Currently, AMDGPU, PPC and LoongArch mark these as legal. The AMDGPU and PPC instructions respect the signed zero behavior. The LoongArch documentation doesn't state, but I'm assuming it also does.
arsenm
requested review from
jayfoad,
RKSimon,
ecnelises,
spavloff,
andykaylor and
jcranmer-intel
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-backend-loongarch Author: Matt Arsenault (arsenm) ChangesPreviously these were declared as having the 2008 behavior, with underspecified signed zero handling. Currently, AMDGPU, PPC and LoongArch mark these as legal. The AMDGPU and PPC instructions respect the signed zero behavior. The LoongArch documentation doesn't state, but I'm assuming it also does. Full diff: https://github.com/llvm/llvm-project/pull/85195.diff 3 Files Affected:
diff --git a/llvm/docs/GlobalISel/GenericOpcode.rst b/llvm/docs/GlobalISel/GenericOpcode.rst
index ac6217d08e6a60..f7d1d5b3edd6a2 100644
--- a/llvm/docs/GlobalISel/GenericOpcode.rst
+++ b/llvm/docs/GlobalISel/GenericOpcode.rst
@@ -521,16 +521,18 @@ The return value of (FMAXNUM 0.0, -0.0) could be either 0.0 or -0.0.
G_FMINNUM_IEEE
^^^^^^^^^^^^^^
-Perform floating-point minimum on two values, following the IEEE-754 2008
-definition. This differs from FMINNUM in the handling of signaling NaNs. If one
-input is a signaling NaN, returns a quiet NaN.
+Perform floating-point minimum on two values, following the IEEE-754
+2019 definition. This differs from FMINNUM in the handling of
+signaling NaNs. If one input is a signaling NaN, returns a quiet
+NaN. This treats -0 as ordered less than +0.
G_FMAXNUM_IEEE
^^^^^^^^^^^^^^
-Perform floating-point maximum on two values, following the IEEE-754 2008
-definition. This differs from FMAXNUM in the handling of signaling NaNs. If one
-input is a signaling NaN, returns a quiet NaN.
+Perform floating-point maximum on two values, following the IEEE-754
+2019 definition. This differs from FMAXNUM in the handling of
+signaling NaNs. If one input is a signaling NaN, returns a quiet
+NaN. This treats -0 as ordered less than +0.
G_FMINIMUM
^^^^^^^^^^
diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h
index 49d51a27e3c0f6..3aa33ee2021250 100644
--- a/llvm/include/llvm/CodeGen/ISDOpcodes.h
+++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h
@@ -971,10 +971,11 @@ enum NodeType {
FMINNUM,
FMAXNUM,
- /// FMINNUM_IEEE/FMAXNUM_IEEE - Perform floating-point minimum or maximum on
- /// two values, following the IEEE-754 2008 definition. This differs from
- /// FMINNUM/FMAXNUM in the handling of signaling NaNs. If one input is a
- /// signaling NaN, returns a quiet NaN.
+ /// FMINNUM_IEEE/FMAXNUM_IEEE - Perform floating-point minimumNumber or
+ /// maximumNumber on two values, following the IEEE-754 2019 definition. This
+ /// differs from FMINNUM/FMAXNUM in the handling of signaling NaNs. If one
+ /// input is a signaling NaN, returns a quiet NaN. These treat -0 as ordered
+ /// less than +0.
FMINNUM_IEEE,
FMAXNUM_IEEE,
diff --git a/llvm/include/llvm/Target/GenericOpcodes.td b/llvm/include/llvm/Target/GenericOpcodes.td
index 67d405ba96fa10..c9513f70a7c613 100644
--- a/llvm/include/llvm/Target/GenericOpcodes.td
+++ b/llvm/include/llvm/Target/GenericOpcodes.td
@@ -795,10 +795,11 @@ def G_FMAXNUM : GenericInstruction {
let isCommutable = true;
}
-// FMINNUM_IEEE/FMAXNUM_IEEE - Perform floating-point minimum or maximum on
-// two values, following the IEEE-754 2008 definition. This differs from
-// FMINNUM/FMAXNUM in the handling of signaling NaNs. If one input is a
-// signaling NaN, returns a quiet NaN.
+// FMINNUM_IEEE/FMAXNUM_IEEE - Perform floating-point minimumNumber or
+// maximumNumber on two values, following the IEEE-754 2019
+// definition. This differs from FMINNUM/FMAXNUM in the handling of
+// signaling NaNs. If one input is a signaling NaN, returns a quiet
+// NaN. These treat -0 as ordered less than +0.
def G_FMINNUM_IEEE : GenericInstruction {
let OutOperandList = (outs type0:$dst);
let InOperandList = (ins type0:$src1, type0:$src2);
|
| Perform floating-point minimum on two values, following the IEEE-754 | ||
| 2019 definition. This differs from FMINNUM in the handling of | ||
| signaling NaNs. If one input is a signaling NaN, returns a quiet | ||
| NaN. This treats -0 as ordered less than +0. |
There was a problem hiding this comment.
Isn't this what G_FMINNUM is supposed to do? The comment for that says that G_FMINNUM has 2019 behaviour, and G_FMINNUM_IEEE has 2008 behaviour. Are you sure we want both to do the same thing? Could we just have G_FMINNUM_IEEE2008 and G_FMINNUM_IEEE2019 instead, so it's less confusing?
There was a problem hiding this comment.
No, because they differ in the behavior for signaling nans. The minnum/maxnum intrinsics were misnamed. They should have been named fmin/fmax, as they follow the libm behavior which ignores signaling nans. The IEEE signaling nan behavior makes no sense, and returns a quiet nan.
Nothing about the nan handling is changing. This PR only adds the stronger signed 0 behavior
There was a problem hiding this comment.
IEEE-754 2019 defines two functions for minimum: minimum and minimumNumber. So it worth mentioning that this node represents just minimumNumber. The described treatment of SNaN is also slightly different from IEEE-754:
minimumNumber(x, y) is x if x < y, y if y < x, and the number if one operand is a number and the
other is a NaN. For this operation, −0 compares less than +0. If x = y and signs are the same it is
either x or y. If both operands are NaNs, a quiet NaN is returned, according to 6.2. If either
operand is a signaling NaN, an invalid operation exception is signaled, but unless both operands
are NaNs, the signaling NaN is otherwise ignored and not converted to a quiet NaN as stated in
6.2 for other operations.
Is this difference intentional?
There was a problem hiding this comment.
Yes. The point of these intermediate nodes is to match hardware behavior for legalization, in which case (for AMDGPU at least) they have the 2008 signaling nan behavior with the stronger 2019 signed zero behavior.
But for the unconstrained versions, the langref hand waves away signaling nan handling saying they may be treated as quiet nans
Maybe these nodes need another rename.
| input is a signaling NaN, returns a quiet NaN. | ||
| Perform floating-point minimum on two values, following the IEEE-754 | ||
| 2019 definition. This differs from FMINNUM in the handling of | ||
| signaling NaNs. If one input is a signaling NaN, returns a quiet |
There was a problem hiding this comment.
Does it? I thought it was supposed to return the number. It should only return a quiet NaN if both inputs are NaNs, no?
|
ping |
|
LGTM. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
The semantics change looks good to me, but I'm still feeling it's (naming?) a little weird: |
Probably, but the point of these intermediates is legalization aid since we don't directly expose them so the point is to match whatever hardware behavior is. We can introduce more variants as necessary |
|
ping |
1 similar comment
|
ping |
Previously these were declared as having the 2008 behavior, with underspecified signed zero handling. Currently, AMDGPU, PPC and LoongArch mark these as legal. The AMDGPU and PPC instructions respect the signed zero behavior. The LoongArch documentation doesn't state, but I'm assuming it also does.