[Intrinsic] Add llvm.minnan and llvm.maxnan instrinsic functions

[tlively]

Summary

These are NaN-propagating versions of llvm.minnum and
llvm.maxnum. These intrinsics lower to the existing fminnan and
fmaxnan instruction selection DAG nodes, so targets that already
support lowering from these nodes should not need to do anything.

WebAssembly in particular requires these intrinsic functions to exist
in order to make its min and max instructions generally available at
the IR level.

[aheejin]

Does this cover all the places we need to handle these new intrinsics? Searching by minnum or maxnum in the llvm directory yields other occurrences in files like lib/Analysis/ValueTracking.cpp, lib/Analysis/VectorUtils.cpp, and lib/Analysis/InstructionSimplify.cpp. (There are more). For test cases as well.

include/llvm/ADT/APFloat.h

1262

Maybe add 1-2 line comments to these two, like other functions?

[tlively]

• Add comments as requested
• Add analogous functionality and tests for every target-independent usage of minnum and maxnum

[dschuff]

docs/LangRef.rst

11596

Does IEEE-754 2018 say anything about which NaN (i.e. is the payload required to be propagated too)?

[sunfish]

docs/LangRef.rst

11596

No, it falls under the general rules for NaN propagation in binary operators (6.2.3). That is, it non-normatively suggests that if two NaN operands are given, one of the payloads be propagated, but it does't give a suggestion as to which.

11599

Another thing that changed in IEEE 754-2018 is that unlike 754-2008's minNum and maxNum, 754-2018's minimum. maximum, minimumNumber, and maximumNumber are defined to treat -0 as if it were less than +0. It'd be nice to adopt this new rule for these new intrinsics, however llvm.minnum already exists and doesn't follow this rule, and existing libm fmin and fmax implementations don't all follow this rule. So, it may not be practical to do so at this time. It'd be useful to mention this discrepancy with the 754-2018 rules here at least though.

[tlively]

• Add notes about exception to IEEE 754-2018 semantics for +0/-0
• Split into multiple commits to make reviewing more pleasant

[tlively]

• Reduce CL to just initial commit, others incoming.

[tlively]

docs/LangRef.rst

11599

Thanks, @sunfish! I've added notes about this.

[tlively]

WebAssembly's handling of +0/-0 *does* match IEEE 754-2018, so these intrinsics (whether target-independent or not) need to be changed to match those semantics as well. I will have to investigate whether the fminnan and fmaxnan ISel DAG nodes are already being used in ways that do not respect these semantics. If they are, we will need new ISel DAG nodes as well.

[sunfish]

In D52450#1248586, @tlively wrote:

WebAssembly's handling of +0/-0 *does* match IEEE 754-2018, so these intrinsics (whether target-independent or not) need to be changed to match those semantics as well. I will have to investigate whether the fminnan and fmaxnan ISel DAG nodes are already being used in ways that do not respect these semantics. If they are, we will need new ISel DAG nodes as well.

It's ok for WebAssembly's semantics to be more constrained than LLVM's. It's the reverse that would be a problem.

[tlively]

In D52450#1248590, @sunfish wrote:

It's ok for WebAssembly's semantics to be more constrained than LLVM's. It's the reverse that would be a problem.

That's true, except that if the semantics of the LLVM intrinsics were not as strict as the semantics of the WebAssembly instructions, then there would be no way for programmers to access WebAssembly's exact semantics at the source level, which is the whole point of implementing these intrinsics at all.

[sunfish]

In D52450#1248681, @tlively wrote:

That's true, except that if the semantics of the LLVM intrinsics were not as strict as the semantics of the WebAssembly instructions, then there would be no way for programmers to access WebAssembly's exact semantics at the source level, which is the whole point of implementing these intrinsics at all.

Oh, right. So in that case, the existing minnum/maxnum intrinsics do have the same problem. That's probably not easy to fix, because it would require changes to existing implementations on other platforms. So we may have to add wasm-specific versions of these intrinsics after all :-/.

[tlively]

In D52450#1248683, @sunfish wrote:

Oh, right. So in that case, the existing minnum/maxnum intrinsics do have the same problem. That's probably not easy to fix, because it would require changes to existing implementations on other platforms. So we may have to add wasm-specific versions of these intrinsics after all :-/.

I don't think adding new intrinsics necessarily means we need to change anything about the old intrinsics. A the C language level, we won't expose builtin functions for wasm that lower to minnum or maxnum, since those don't match wasm semantics. To prevent people from assuming that the only difference between minnum/maxnum and the new intrinsics is NaN propagation, I propose we name the new instrinsics minimum/maximum (after the new IEEE operations) rather than minnan/maxnan.

[tlively]

Abandoned in favor of D52764.

docs/LangRef.rst

 
 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``).
 
+'``llvm.minnan.*``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Syntax:
+"""""""
+
+This is an overloaded intrinsic. You can use ``llvm.minnan`` on any
+floating-point or vector of floating-point type. Not all targets support
+all types however.
+
+::
+
+      declare float     @llvm.minnan.f32(float %Val0, float %Val1)
+      declare double    @llvm.minnan.f64(double %Val0, double %Val1)
+      declare x86_fp80  @llvm.minnan.f80(x86_fp80 %Val0, x86_fp80 %Val1)
+      declare fp128     @llvm.minnan.f128(fp128 %Val0, fp128 %Val1)
+      declare ppc_fp128 @llvm.minnan.ppcf128(ppc_fp128 %Val0, ppc_fp128 %Val1)
+
+Overview:
+"""""""""
+
+The '``llvm.minnan.*``' intrinsics return the minimum of the two
+arguments, propagating NaNs.
+
+
+Arguments:
+""""""""""
+
+The arguments and return value are floating-point numbers of the same
+type.
+
+Semantics:
+""""""""""
+If either operand is a NaN, returns NaN. If the operands compare
+equal, returns a value that compares equal to both operands. This
+means that llvm.minnan(+/-0.0, +/-0.0) could return either -0.0 or
+0.0. Note that this is different from IEEE 754-2018 semantics, which
+require that -0 compare less than +0 for this operation.
+
+'``llvm.maxnan.*``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Syntax:
+"""""""
+
+This is an overloaded intrinsic. You can use ``llvm.maxnan`` on any
+floating-point or vector of floating-point type. Not all targets support
+all types however.
+
+::
+
+      declare float     @llvm.maxnan.f32(float  %Val0, float  %Val1l)
+      declare double    @llvm.maxnan.f64(double %Val0, double %Val1)
+      declare x86_fp80  @llvm.maxnan.f80(x86_fp80  %Val0, x86_fp80  %Val1)
+      declare fp128     @llvm.maxnan.f128(fp128 %Val0, fp128 %Val1)
+      declare ppc_fp128 @llvm.maxnan.ppcf128(ppc_fp128  %Val0, ppc_fp128  %Val1)
+
+Overview:
+"""""""""
+
+The '``llvm.maxnan.*``' intrinsics return the maximum of the two
+arguments, propagating NaNs.
+
+
+Arguments:
+""""""""""
+
+The arguments and return value are floating-point numbers of the same
+type.
+
+Semantics:
+""""""""""
+If either operand is a NaN, returns NaN. If the operands compare
+equal, returns a value that compares equal to both operands. This
+means that llvm.maxnan(+/-0.0, +/-0.0) could return either -0.0 or
+0.0.Note that this is different from IEEE 754-2018 semantics, which
+require that -0 compare less than +0 for this operation.
+
 '``llvm.copysign.*``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 """""""
 
 This is an overloaded intrinsic. You can use ``llvm.copysign`` on any
 floating-point or vector of floating-point type. Not all targets support

include/llvm/ADT/APFloat.h

 inline APFloat maxnum(const APFloat &A, const APFloat &B) {
   if (A.isNaN())
     return B;
   if (B.isNaN())
     return A;
   return (A.compare(B) == APFloat::cmpLessThan) ? B : A;
 }
 
+/// Returns the smaller of the 2 arguments if both are not Nan. If either
+/// argument is a NaN, return the NaN. Implements IEEE 754-2018 `minimum`
+/// semantics, except that -0 does not necessarily compare less than +0.
+LLVM_READONLY
+inline APFloat minnan(const APFloat &A, const APFloat &B) {
+  if (A.isNaN())
+    return A;
+  if (B.isNaN())
+    return B;
+  return (B.compare(A) == APFloat::cmpLessThan) ? B : A;
+}
+
+/// Returns the larger of the 2 arguments if both are not Nan. If either
+/// argument is a NaN, return the NaN. Implements IEEE 754-2018 `maximum`
+/// semantics, except that -0 does not necessarily compare less than +0.
+LLVM_READONLY
+inline APFloat maxnan(const APFloat &A, const APFloat &B) {
+  if (A.isNaN())
+    return A;
+  if (B.isNaN())
+    return B;
+  return (A.compare(B) == APFloat::cmpLessThan) ? B : A;
+}
+
 } // namespace llvm
 
 #undef APFLOAT_DISPATCH_ON_SEMANTICS
 #endif // LLVM_ADT_APFLOAT_H

include/llvm/IR/IRBuilder.h

CallInst *CreateMinNum(Value *LHS, Value *RHS, const Twine &Name = "") {
     return CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS, Name);
   }
 
   /// Create call to the maxnum intrinsic.
   CallInst *CreateMaxNum(Value *LHS, Value *RHS, const Twine &Name = "") {
     return CreateBinaryIntrinsic(Intrinsic::maxnum, LHS, RHS, Name);
   }
 
+  /// Create call to the minnan intrinsic.
+  CallInst *CreateMinNan(Value *LHS, Value *RHS, const Twine &Name = "") {
+    return CreateBinaryIntrinsic(Intrinsic::minnan, LHS, RHS, Name);
+  }
+
+  /// Create call to the maxnan intrinsic.
+  CallInst *CreateMaxNan(Value *LHS, Value *RHS, const Twine &Name = "") {
+    return CreateBinaryIntrinsic(Intrinsic::maxnan, LHS, RHS, Name);
+  }
+
 private:
   /// Create a call to a masked intrinsic with given Id.
   CallInst *CreateMaskedIntrinsic(Intrinsic::ID Id, ArrayRef<Value *> Ops,
                                   ArrayRef<Type *> OverloadedTypes,
                                   const Twine &Name = "");
 
   Value *getCastedInt8PtrValue(Value *Ptr);
 };

include/llvm/IR/Intrinsics.td

 def int_minnum : Intrinsic<[llvm_anyfloat_ty],
   [LLVMMatchType<0>, LLVMMatchType<0>],
   [IntrNoMem, IntrSpeculatable, Commutative]
 >;
 def int_maxnum : Intrinsic<[llvm_anyfloat_ty],
   [LLVMMatchType<0>, LLVMMatchType<0>],
   [IntrNoMem, IntrSpeculatable, Commutative]
 >;
+def int_minnan : Intrinsic<[llvm_anyfloat_ty],
+  [LLVMMatchType<0>, LLVMMatchType<0>],
+  [IntrNoMem, IntrSpeculatable, Commutative]
+>;
+def int_maxnan : Intrinsic<[llvm_anyfloat_ty],
+  [LLVMMatchType<0>, LLVMMatchType<0>],
+  [IntrNoMem, IntrSpeculatable, Commutative]
+>;
 
 // NOTE: these are internal interfaces.
 def int_setjmp     : Intrinsic<[llvm_i32_ty],  [llvm_ptr_ty]>;
 def int_longjmp    : Intrinsic<[], [llvm_ptr_ty, llvm_i32_ty], [IntrNoReturn]>;
 def int_sigsetjmp  : Intrinsic<[llvm_i32_ty] , [llvm_ptr_ty, llvm_i32_ty]>;
 def int_siglongjmp : Intrinsic<[], [llvm_ptr_ty, llvm_i32_ty], [IntrNoReturn]>;
 
 // Internal interface for object size checking

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

unsigned Opc =
         I.hasNoNaNs() && TLI.isOperationLegalOrCustom(ISD::FMAXNAN, VT)
             ? ISD::FMAXNAN
             : ISD::FMAXNUM;
     setValue(&I, DAG.getNode(Opc, sdl, VT,
                              getValue(I.getArgOperand(0)),
                              getValue(I.getArgOperand(1))));
     return nullptr;
   }
+  case Intrinsic::minnan:
+    setValue(&I, DAG.getNode(ISD::FMINNAN, sdl,
+                             getValue(I.getArgOperand(0)).getValueType(),
+                             getValue(I.getArgOperand(0)),
+                             getValue(I.getArgOperand(1))));
+    return nullptr;
+  case Intrinsic::maxnan:
+    setValue(&I, DAG.getNode(ISD::FMAXNAN, sdl,
+                             getValue(I.getArgOperand(0)).getValueType(),
+                             getValue(I.getArgOperand(0)),
+                             getValue(I.getArgOperand(1))));
+    return nullptr;
   case Intrinsic::copysign:
     setValue(&I, DAG.getNode(ISD::FCOPYSIGN, sdl,
                              getValue(I.getArgOperand(0)).getValueType(),
                              getValue(I.getArgOperand(0)),
                              getValue(I.getArgOperand(1))));
     return nullptr;
   case Intrinsic::fma:
     setValue(&I, DAG.getNode(ISD::FMA, sdl,

unittests/IR/IRBuilderTest.cpp

TEST_F(IRBuilderTest, Intrinsics) {
   Call = Builder.CreateMinNum(V, V);
   II = cast<IntrinsicInst>(Call);
   EXPECT_EQ(II->getIntrinsicID(), Intrinsic::minnum);
 
   Call = Builder.CreateMaxNum(V, V);
   II = cast<IntrinsicInst>(Call);
   EXPECT_EQ(II->getIntrinsicID(), Intrinsic::maxnum);
 
+  Call = Builder.CreateMinNan(V, V);
+  II = cast<IntrinsicInst>(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::minnan);
+
+  Call = Builder.CreateMaxNan(V, V);
+  II = cast<IntrinsicInst>(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::maxnan);
+
   Call = Builder.CreateIntrinsic(Intrinsic::readcyclecounter);
   II = cast<IntrinsicInst>(Call);
   EXPECT_EQ(II->getIntrinsicID(), Intrinsic::readcyclecounter);
 }
 
 TEST_F(IRBuilderTest, Lifetime) {
   IRBuilder<> Builder(BB);
   AllocaInst *Var1 = Builder.CreateAlloca(Builder.getInt8Ty());