[clang][Interp] Implement __builtin_isnan()

[tbaeder]

Summary

Add a way to pop arguments based on the call expression instead of the function and use that.

clang/test/Sema/constant-builtins-fmin.cpp is the promised test form the other reviews.

[cor3ntin]

clang/lib/AST/Interp/Function.h

180

You can compare getBuiltinID(), which is better than using strings, it's what we seem to be doing elsewhere

clang/lib/AST/Interp/InterpBuiltin.cpp

173

There are only a few functions created by SemaBuiltinFPClassification - only two of which are not unary. this looks fine, fpclassify and isfpclass can have their own logic.
I don't suppose there is a way to assert we have one argument there?

[tbaeder]

clang/lib/AST/Interp/Function.h

180

Yeah but that still sucks. Is there no way to query whether the builtin has t set ("signature is meaningless, use custom typechecking"), regardless of the ID or name?

clang/lib/AST/Interp/InterpBuiltin.cpp

173

We could (and maybe want) to pass along the CallExpr, but the arguments and their number are already checked in Sema.

[cor3ntin]

clang/lib/AST/Interp/Function.h

180

AstContext.BuiltinInfo.hasCustomTypechecking(F->getBuiltinID())) seems to do that but i have no clue if you could use it on all functions without running into issues

[tbaeder]

clang/lib/AST/Interp/Function.h

180

That works indeed, thanks.

[cor3ntin]

clang/lib/AST/Interp/InterpBuiltin.cpp

170–172

I would remove the fixme, given you already implemented all the floating point builtins

clang/lib/AST/Interp/Function.h

public:
 
   bool hasThisPointer() const { return HasThisPointer; }
 
   /// Checks if the function already has a body attached.
   bool hasBody() const { return HasBody; }
 
   unsigned getBuiltinID() const { return F->getBuiltinID(); }
 
+  bool isBuiltin() const { return F->getBuiltinID() != 0; }
+
+  /// Does this function need its arguments to be classified at runtime
+  /// rather than at bytecode-compile-time?
+  bool needsRuntimeArgPop(const ASTContext &Ctx) const;
+
   unsigned getNumParams() const { return ParamTypes.size(); }
 
   unsigned getParamOffset(unsigned ParamIndex) const {
     return ParamOffsets[ParamIndex];
   }
 
 private:
   /// Construct a function representing an actual function.
   Function(Program &P, const FunctionDecl *F, unsigned ArgSize,

clang/lib/AST/Interp/Function.cpp

 //===--- Function.h - Bytecode function for the VM --------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Function.h"
-#include "Program.h"
 #include "Opcode.h"
+#include "Program.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/Basic/Builtins.h"
 
 using namespace clang;
 using namespace clang::interp;
 
 Function::Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
                    llvm::SmallVectorImpl<PrimType> &&ParamTypes,
                    llvm::DenseMap<unsigned, ParamDescriptor> &&Params,
                    llvm::SmallVectorImpl<unsigned> &&ParamOffsets,
SourceInfo Function::getSource(CodePtr PC) const {
   return It->second;
 }
 
 bool Function::isVirtual() const {
   if (auto *M = dyn_cast<CXXMethodDecl>(F))
     return M->isVirtual();
   return false;
 }
+
+bool Function::needsRuntimeArgPop(const ASTContext &Ctx) const {
+  if (!isBuiltin())
+    return false;
+  return Ctx.BuiltinInfo.hasCustomTypechecking(getBuiltinID());
+}

clang/lib/AST/Interp/Interp.h

 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F);
 
 enum class ArithOp { Add, Sub };
 
 //===----------------------------------------------------------------------===//
 // Returning values
 //===----------------------------------------------------------------------===//
 
+/// Pop arguments of builtins defined as func-name(...).
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC);
+
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   const T &Ret = S.Stk.pop<T>();
 
   // Make sure returned pointers are live. We might be trying to return a
   // pointer or reference to a local variable.
   // Just return false, since a diagnostic has already been emitted in Sema.
   if constexpr (std::is_same_v<T, Pointer>) {
     // FIXME: We could be calling isLive() here, but the emitted diagnostics
     // seem a little weird, at least if the returned expression is of
     // pointer type.
     if (!Ret.isLive())
       return false;
   }
 
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
+  if (!S.checkingPotentialConstantExpression() || S.Current->Caller) {
+    // Certain builtin functions are declared as func-name(...), so the
+    // parameters are checked in Sema and only available through the CallExpr.
+    // The interp::Function we create for them has 0 parameters, so we need to
+    // remove them from the stack by checking the CallExpr.
+    if (S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()))
+      popBuiltinArgs(S, PC);
+    else
       S.Current->popArgs();
+  }
 
   if (InterpFrame *Caller = S.Current->Caller) {
     PC = S.Current->getRetPC();
     delete S.Current;
     S.Current = Caller;
     S.Stk.push<T>(Ret);
   } else {
     delete S.Current;

clang/lib/AST/Interp/Interp.cpp

if (auto ID = Ptr.getDeclID()) {
     return false;
   }
   return true;
 }
 
 namespace clang {
 namespace interp {
 
+bool popBuiltinArgs(InterpState &S, CodePtr OpPC) {
+  assert(S.Current && S.Current->getFunction()->needsRuntimeArgPop(S.getCtx()));
+  const Expr *E = S.Current->getExpr(OpPC);
+  assert(isa<CallExpr>(E));
+  const CallExpr *CE = cast<CallExpr>(E);
+  for (const auto *A : llvm::reverse(CE->arguments())) {
+    PrimType Ty = S.getContext().classify(A->getType()).value_or(PT_Ptr);
+    TYPE_SWITCH(Ty, S.Stk.discard<T>());
+  }
+  return true;
+}
+
 bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   if (!Ptr.isExtern())
     return true;
 
   if (!S.checkingPotentialConstantExpression()) {
     const auto *VD = Ptr.getDeclDesc()->asValueDecl();
     const SourceInfo &Loc = S.Current->getSource(OpPC);
     S.FFDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;

clang/lib/AST/Interp/InterpBuiltin.cpp

else if (LHS.isNan() || RHS < LHS)
     Result = RHS;
   else
     Result = LHS;
 
   S.Stk.push<Floating>(Result);
   return true;
 }
 
+/// Defined as __builtin_isnan(...), to accommodate the fact that it can
+/// take a float, double, long double, etc.
+/// But for us, that's all a Floating anyway.
+static bool interp__builtin_isnan(InterpState &S, CodePtr OpPC,
+                                  const InterpFrame *Frame, const Function *F) {
+  const Floating &Arg = S.Stk.peek<Floating>();
+
+  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isNan()));
+  return true;
+}
+
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   InterpFrame *Frame = S.Current;
   APValue Dummy;
 
   switch (F->getBuiltinID()) {
   case Builtin::BI__builtin_is_constant_evaluated:
     S.Stk.push<Boolean>(Boolean::from(S.inConstantContext()));
     return Ret<PT_Bool>(S, OpPC, Dummy);
bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F) {
   case Builtin::BI__builtin_fminf:
   case Builtin::BI__builtin_fminl:
   case Builtin::BI__builtin_fminf16:
   case Builtin::BI__builtin_fminf128:
     if (interp__builtin_fmin(S, OpPC, Frame, F))
       return Ret<PT_Float>(S, OpPC, Dummy);
     break;
 
+  case Builtin::BI__builtin_isnan:
+    if (interp__builtin_isnan(S, OpPC, Frame, F))
+      return Ret<PT_Sint32>(S, OpPC, Dummy);
+    break;
+
   default:
     return false;
   }
 
   return false;
 }
 
 } // namespace interp
 } // namespace clang

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

 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s
 // expected-no-diagnostics
 
 constexpr double NaN = __builtin_nan("");
 constexpr double Inf = __builtin_inf();
 constexpr double NegInf = -__builtin_inf();
 
 #define FMIN_TEST_SIMPLE(T, FUNC)                           \
     static_assert(T(1.2345) == FUNC(T(1.2345), T(6.7890))); \