[libc-commits] [PATCH] D90906: [libc] Add implementations of fdim[f|l].

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/test/src/math/FDimTest.h:24
+  void testNaNArg(FuncPtr func) {
+    DECLARE_SPECIAL_CONSTANTS(T)
+    EXPECT_FP_EQ(nan, func(nan, inf));
----------------
lntue wrote:
> sivachandra wrote:
> > Can you move this declaration out of the method? This way, other methods can also use them without redeclarations?
> FPBits class cannot be constexpr yet, so we cannot have these constants as static members of the class yet.  I moved them out as const members instead.
LGTM.


================
Comment at: libc/utils/FPUtil/BasicOperations.h:78
+
+  return (x > y ? x - y : 0);
+}
----------------
lntue wrote:
> sivachandra wrote:
> > The operation `x - y` can lead to overflow or underflow. Shouldn't we detect that and return the appropriate value as specified by the standard? Or, should we assume that the machine instruction does the right thing?
> We are assuming CPU's supporting IEEE 754 arithmetic, so the machine instruction will do the right thing.  For example the spec specified to return HUGE_VAL in case of overflow, which is resolved to inf for IEEE 754 standard.
SGTM. So, in general, out position is:

  If we use a language operator for a floating point operation, then we assume that the machine instruction (or instructions) implementing the operator conform to the IEEE-754 standard.

Does that sound reasonable?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90906/new/

https://reviews.llvm.org/D90906