[Clang] Support constexpr for builtin fmax, fmin, ilogb, logb, scalbn

[Izaron]

Summary

Support constexpr versions of builtin_fmax, builtin_fmin,
builtin_ilogb, builtin_logb, __builtin_scalbn and their
variations.

[Izaron]

libc++ doesn't support constexpr-related patches from C++20 in <complex>: see issue https://github.com/llvm/llvm-project/issues/55370

I reviewed the code in <complex> and found out that since we use a dozen of math functions, we need to support more constexpr builtin math function.

I implemented constexpr versions for five functions. This will be enough to implement a 2017 paper: https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0415r1.html (in a new pull request)

(Also there is a 2019 paper with more constexpr functions for <complex>: https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2019/p1383r0.pdf)

Also builtin implementations will be useful for new C++23 constexpr functions: for example https://en.cppreference.com/w/cpp/numeric/math/scalbn

[Izaron]

clang/test/Sema/constant-builtins-math.cpp

17–18

runtime version of __builtin_ilogb/std::ilogb returns INT_MIN (-2147483647) on this argument, while constexpr version returns INT_MIN+1 🤔

This constant is here:
https://llvm.org/doxygen/APFloat_8cpp_source.html#l04187
https://llvm.org/doxygen/APFloat_8h_source.html#l00229
(I guess we shouldn't change this)

17–18

quick fix: INT_MIN is -2147483648, INT_MIN+1 is 2147483647

[shafik]

llvm/lib/Support/APFloat.cpp

4206

This is the format as documented by clang-tidy bugprone argument comment see here: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html

IIRC it depends on strict mode but I would just avoid the whitespace.

[Izaron]

Removed whitespaces in comments.

P.S. I wonder if smaller review requests will make things better? Say a review request for fmax+fmin, then a new one for ilogb+logb, then for scalbn.
This may lead to a more understandable and robust review process =)

[efriedma]

These patches are hard to review, yes; the reviewer has to go through and verify for each function that the substitutes you've written actually match the C library semantics. (It's very easy to make mistakes with that sort of thing.)

On that front, I'm pretty sure the NaN handling in your fmin/fmax implementations are wrong.

[Izaron]

In D134136#3801495, @efriedma wrote:

These patches are hard to review, yes; the reviewer has to go through and verify for each function that the substitutes you've written actually match the C library semantics. (It's very easy to make mistakes with that sort of thing.)

On that front, I'm pretty sure the NaN handling in your fmin/fmax implementations are wrong.

Thanks! I'm abandoning this patch, will make new smaller but better patches.

each function that the substitutes you've written actually match the C library semantics

I think this can be verified with libc++/libc tests that would substitute either std::fmax or (constant) __builtin_fmax on same checks and assert that they behave in the same way.

clang/docs/LanguageExtensions.rst

 * ``__builtin_clzs``
 * ``__builtin_ctz``
 * ``__builtin_ctzl``
 * ``__builtin_ctzll``
 * ``__builtin_ctzs``
 * ``__builtin_ffs``
 * ``__builtin_ffsl``
 * ``__builtin_ffsll``
+* ``__builtin_fmax``
+* ``__builtin_fmin``
 * ``__builtin_fpclassify``
+* ``__builtin_ilogb``
 * ``__builtin_inf``
 * ``__builtin_isinf``
 * ``__builtin_isinf_sign``
 * ``__builtin_isfinite``
 * ``__builtin_isnan``
 * ``__builtin_isnormal``
+* ``__builtin_logb``
 * ``__builtin_nan``
 * ``__builtin_nans``
 * ``__builtin_parity``
 * ``__builtin_parityl``
 * ``__builtin_parityll``
 * ``__builtin_popcount``
 * ``__builtin_popcountl``
 * ``__builtin_popcountll``
 * ``__builtin_rotateleft8``
 * ``__builtin_rotateleft16``
 * ``__builtin_rotateleft32``
 * ``__builtin_rotateleft64``
 * ``__builtin_rotateright8``
 * ``__builtin_rotateright16``
 * ``__builtin_rotateright32``
 * ``__builtin_rotateright64``
+* ``__builtin_scalbn``
 
 The following x86-specific intrinsics can be used in constant expressions:
 
 * ``_bit_scan_forward``
 * ``_bit_scan_reverse``
 * ``__bsfd``
 * ``__bsfq``
 * ``__bsrd``

clang/lib/AST/ExprConstant.cpp

if (BuiltinOp == Builtin::BI__builtin_add_overflow ||
       Result = Temp;
     }
 
     APValue APV{Result};
     if (!handleAssignment(Info, E, ResultLValue, ResultType, APV))
       return false;
     return Success(DidOverflow, E);
   }
+
+  case Builtin::BI__builtin_ilogb:
+  case Builtin::BI__builtin_ilogbf:
+  case Builtin::BI__builtin_ilogbl:
+  case Builtin::BI__builtin_ilogbf128: {
+    APFloat F(0.0);
+    if (!EvaluateFloat(E->getArg(0), F, Info))
+      return false;
+    APSInt Logb(llvm::APInt(32, ilogb(F), /*isSigned=*/true),
+                /*isUnsigned=*/false);
+    Result = APValue(std::move(Logb));
+    return true;
+  }
   }
 }
 
 /// Determine whether this is a pointer past the end of the complete
 /// object referred to by the lvalue.
 static bool isOnePastTheEndOfCompleteObject(const ASTContext &Ctx,
                                             const LValue &LV) {
   // A null pointer can be viewed as being "past the end" but we don't
bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
   case Builtin::BI__builtin_copysignf128: {
     APFloat RHS(0.);
     if (!EvaluateFloat(E->getArg(0), Result, Info) ||
         !EvaluateFloat(E->getArg(1), RHS, Info))
       return false;
     Result.copySign(RHS);
     return true;
   }
+
+  case Builtin::BI__builtin_fmax:
+  case Builtin::BI__builtin_fmaxf:
+  case Builtin::BI__builtin_fmaxl:
+  case Builtin::BI__builtin_fmaxf16:
+  case Builtin::BI__builtin_fmaxf128: {
+    APFloat RHS(0.);
+    if (!EvaluateFloat(E->getArg(0), Result, Info) ||
+        !EvaluateFloat(E->getArg(1), RHS, Info))
+      return false;
+    if (RHS > Result)
+      Result = RHS;
+    return true;
+  }
+
+  case Builtin::BI__builtin_fmin:
+  case Builtin::BI__builtin_fminf:
+  case Builtin::BI__builtin_fminl:
+  case Builtin::BI__builtin_fminf16:
+  case Builtin::BI__builtin_fminf128: {
+    APFloat RHS(0.);
+    if (!EvaluateFloat(E->getArg(0), Result, Info) ||
+        !EvaluateFloat(E->getArg(1), RHS, Info))
+      return false;
+    if (RHS < Result)
+      Result = RHS;
+    return true;
+  }
+
+  case Builtin::BI__builtin_logb:
+  case Builtin::BI__builtin_logbf:
+  case Builtin::BI__builtin_logbl:
+  case Builtin::BI__builtin_logbf128: {
+    APFloat F(0.0);
+    if (!EvaluateFloat(E->getArg(0), F, Info))
+      return false;
+    Result = logb(F);
+    return true;
+  }
+
+  case Builtin::BI__builtin_scalbn:
+  case Builtin::BI__builtin_scalbnf:
+  case Builtin::BI__builtin_scalbnl:
+  case Builtin::BI__builtin_scalbnf128: {
+    APFloat X(0.0);
+    APSInt Exp;
+    if (!EvaluateFloat(E->getArg(0), X, Info) ||
+        !EvaluateInteger(E->getArg(1), Exp, Info))
+      return false;
+    Result = scalbn(X, Exp.getExtValue(), APFloat::rmNearestTiesToEven);
+    return true;
+  }
   }
 }
 
 bool FloatExprEvaluator::VisitUnaryReal(const UnaryOperator *E) {
   if (E->getSubExpr()->getType()->isAnyComplexType()) {
     ComplexValue CV;
     if (!EvaluateComplex(E->getSubExpr(), CV, Info))
       return false;

clang/test/Sema/constant-builtins-math.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+static_assert(__builtin_fmax(12.34, 56.78) == 56.78);
+static_assert(__builtin_fmaxf(12.34f, 56.78f) == 56.78f);
+static_assert(__builtin_fmaxl(12.34, 56.78) == 56.78);
+static_assert(__builtin_fmaxf16(12.0, 56.0) == 56.0);
+static_assert(__builtin_fmaxf128(12.34, 56.78) == 56.78);
+
+static_assert(__builtin_fmin(12.34, 56.78) == 12.34);
+static_assert(__builtin_fminf(12.34f, 56.78f) == 12.34f);
+static_assert(__builtin_fminl(12.34, 56.78) == 12.34);
+static_assert(__builtin_fminf16(12.0, 56.0) == 12.0);
+static_assert(__builtin_fminf128(12.34, 56.78) == 12.34);
+
+// TODO: investigate why this is `INT_MIN + 1` instead of `INT_MIN`
+static_assert(__builtin_ilogb(0.0) == -2147483647);
+static_assert(__builtin_ilogb(__builtin_inf()) == 2147483647);
+static_assert(__builtin_ilogb(__builtin_nan("")) == -2147483648);
+static_assert(__builtin_ilogb(1.0) == 0);
+static_assert(__builtin_ilogb(32.0) == 5);
+static_assert(__builtin_ilogb(1024.0) == 10);
+static_assert(__builtin_ilogb(-1024.0) == 10);
+static_assert(__builtin_ilogb(0.1) == -4);
+static_assert(__builtin_ilogb(0.0000000000001) == -44);
+
+static_assert(__builtin_logb(0.0) == -__builtin_inf());
+static_assert(__builtin_logb(__builtin_inf()) == __builtin_inf());
+static_assert(__builtin_isnan(__builtin_logb(__builtin_nan(""))));
+static_assert(__builtin_logb(1.0) == 0.0);
+static_assert(__builtin_logb(32.0) == 5.0);
+static_assert(__builtin_logb(1024.0) == 10.0);
+static_assert(__builtin_logb(-1024.0) == 10.0);
+static_assert(__builtin_logb(0.1) == -4.0);
+static_assert(__builtin_logb(0.0000000000001) == -44.0);
+
+static_assert(__builtin_scalbn(12.5, 0) == 12.5);
+static_assert(__builtin_scalbn(12.5, 1) == 25);
+static_assert(__builtin_scalbn(12.5, 3) == 100.0);
+static_assert(__builtin_scalbn(0.0, 100) == 0);
+static_assert(__builtin_scalbn(__builtin_inf(), 100) == __builtin_inf());
+static_assert(__builtin_isnan(__builtin_scalbn(__builtin_nan(""), 100)));

llvm/include/llvm/ADT/APFloat.h

private:
   fltCategory category : 3;
 
   /// Sign bit of the number.
   unsigned int sign : 1;
 };
 
 hash_code hash_value(const IEEEFloat &Arg);
 int ilogb(const IEEEFloat &Arg);
+IEEEFloat logb(const IEEEFloat &Arg);
 IEEEFloat scalbn(IEEEFloat X, int Exp, IEEEFloat::roundingMode);
 IEEEFloat frexp(const IEEEFloat &Val, int &Exp, IEEEFloat::roundingMode RM);
 
 // This mode implements more precise float in terms of two APFloats.
 // The interface and layout is designed for arbitrary underlying semantics,
 // though currently only PPCDoubleDouble semantics are supported, whose
 // corresponding underlying semantics are IEEEdouble.
 class DoubleAPFloat final : public APFloatBase {
public:
   void dump() const;
 
   bool getExactInverse(APFloat *inv) const {
     APFLOAT_DISPATCH_ON_SEMANTICS(getExactInverse(inv));
   }
 
   friend hash_code hash_value(const APFloat &Arg);
   friend int ilogb(const APFloat &Arg) { return ilogb(Arg.getIEEE()); }
+  friend APFloat logb(const APFloat &Arg) {
+    return APFloat(logb(Arg.getIEEE()), Arg.getSemantics());
+  }
   friend APFloat scalbn(APFloat X, int Exp, roundingMode RM);
   friend APFloat frexp(const APFloat &X, int &Exp, roundingMode RM);
   friend IEEEFloat;
   friend DoubleAPFloat;
 };
 
 /// See friend declarations above.
 ///

llvm/lib/Support/APFloat.cpp

int ilogb(const IEEEFloat &Arg) {
   IEEEFloat Normalized(Arg);
   int SignificandBits = Arg.getSemantics().precision - 1;
 
   Normalized.exponent += SignificandBits;
   Normalized.normalize(IEEEFloat::rmNearestTiesToEven, lfExactlyZero);
   return Normalized.exponent - SignificandBits;
 }
 
+IEEEFloat logb(const IEEEFloat &Arg) {
+  IEEEFloat Result(Arg.getSemantics());
+  if (Arg.isNaN())
+    Result.makeNaN();
+  else if (Arg.isZero())
+    Result.makeInf(/*Negative=*/true);
+  else if (Arg.isInfinity())
+    Result.makeInf(/*Negative=*/false);
+  else
+    Result = IEEEFloat(static_cast<double>(ilogb(Arg)));
+  return Result;
+}
+
 IEEEFloat scalbn(IEEEFloat X, int Exp, IEEEFloat::roundingMode RoundingMode) {
   auto MaxExp = X.getSemantics().maxExponent;
   auto MinExp = X.getSemantics().minExponent;
 
   // If Exp is wildly out-of-scale, simply adding it to X.exponent will
   // overflow; clamp it to a safe range before adding, but ensure that the range
   // is large enough that the clamp does not change the result. The range we
   // need to support is the difference between the largest possible exponent and