Commit f584f04d authored by Nandor Licker's avatar Nandor Licker
Browse files

[ConstExprPreter] Removed the flag forcing the use of the interpreter 

Summary:
Removed the ```-fforce-experimental-new-constant-interpreter flag```, leaving
only the ```-fexperimental-new-constant-interpreter``` one. The interpreter
now always emits an error on an unsupported feature.

Allowing the interpreter to bail out would require a mapping from APValue to
interpreter memory, which will not be necessary in the final version. It is
more sensible to always emit an error if the interpreter fails.

Reviewers: jfb, Bigcheese, rsmith, dexonsmith

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70071
parent 9283681e

clang/docs/ConstantInterpreter.rst

+1 −2
Original line number Diff line number Diff line
@@ -10,8 +10,7 @@ Introduction 


The constexpr interpreter aims to replace the existing tree evaluator in clang, improving performance on constructs which are executed inefficiently by the evaluator. The interpreter is activated using the following flags:



* ``-fexperimental-new-constant-interpreter`` enables the interpreter, falling back to the evaluator for unsupported features

* ``-fforce-experimental-new-constant-interpreter`` forces the use of the interpreter, bailing out if an unsupported feature is encountered

* ``-fexperimental-new-constant-interpreter`` enables the interpreter, emitting an error if an unsupported feature is encountered



Bytecode Compilation

====================

clang/include/clang/Basic/LangOptions.def

+0 −2
Original line number Diff line number Diff line
@@ -297,8 +297,6 @@ BENIGN_LANGOPT(ConstexprStepLimit, 32, 1048576, 
               "maximum constexpr evaluation steps")

BENIGN_LANGOPT(EnableNewConstInterp, 1, 0,

               "enable the experimental new constant interpreter")

BENIGN_LANGOPT(ForceNewConstInterp, 1, 0,

               "force the use of the experimental new constant interpreter")

BENIGN_LANGOPT(BracketDepth, 32, 256,

               "maximum bracket nesting depth")

BENIGN_LANGOPT(NumLargeByValueCopy, 32, 0,

clang/include/clang/Driver/Options.td

+0 −2
Original line number Diff line number Diff line
@@ -850,8 +850,6 @@ def fconstexpr_depth_EQ : Joined<["-"], "fconstexpr-depth=">, Group<f_Group>; 
def fconstexpr_steps_EQ : Joined<["-"], "fconstexpr-steps=">, Group<f_Group>;

def fexperimental_new_constant_interpreter : Flag<["-"], "fexperimental-new-constant-interpreter">, Group<f_Group>,

  HelpText<"Enable the experimental new constant interpreter">, Flags<[CC1Option]>;

def fforce_experimental_new_constant_interpreter : Flag<["-"], "fforce-experimental-new-constant-interpreter">, Group<f_Group>,

  HelpText<"Force the use of the experimental new constant interpreter, failing on missing features">, Flags<[CC1Option]>;

def fconstexpr_backtrace_limit_EQ : Joined<["-"], "fconstexpr-backtrace-limit=">,

                                    Group<f_Group>;

def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, Group<f_clang_Group>, Flags<[NoArgumentUnused, CoreOption]>,

clang/lib/AST/ExprConstant.cpp

+45 −73
Original line number Diff line number Diff line
@@ -763,11 +763,8 @@ namespace { 
    /// we will evaluate.

    unsigned StepsLeft;













    /// Force the use of the experimental new constant interpreter, bailing out













    /// with an error if a feature is not supported.













    bool ForceNewConstInterp;

























    /// Enable the experimental new constant interpreter.













    /// Enable the experimental new constant interpreter. If an expression is













    /// not supported by the interpreter, an error is triggered.















    bool EnableNewConstInterp;





























    /// BottomFrame - The frame in which evaluation started. This must be










@@ -922,9 +919,7 @@ namespace {













        : Ctx(const_cast<ASTContext &>(C)), EvalStatus(S), CurrentCall(nullptr),















          CallStackDepth(0), NextCallIndex(1),















          StepsLeft(C.getLangOpts().ConstexprStepLimit),













          ForceNewConstInterp(C.getLangOpts().ForceNewConstInterp),













          EnableNewConstInterp(ForceNewConstInterp ||













                               C.getLangOpts().EnableNewConstInterp),













          EnableNewConstInterp(C.getLangOpts().EnableNewConstInterp),















          BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),















          EvaluatingDecl((const ValueDecl *)nullptr),















          EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),










@@ -13401,17 +13396,9 @@ static bool EvaluateInPlace(APValue &Result, EvalInfo &Info, const LValue &This,













/// lvalue-to-rvalue cast if it is an lvalue.















static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {















  if (Info.EnableNewConstInterp) {













    auto &InterpCtx = Info.Ctx.getInterpContext();













    switch (InterpCtx.evaluateAsRValue(Info, E, Result)) {













    case interp::InterpResult::Success:













      return true;













    case interp::InterpResult::Fail:













    if (!Info.Ctx.getInterpContext().evaluateAsRValue(Info, E, Result))















      return false;













    case interp::InterpResult::Bail:













      break;













    }













  }

























  } else {















    if (E->getType().isNull())















      return false;























@@ -13427,6 +13414,7 @@ static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {













      if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))















        return false;















    }













  }





























  // Check this core constant expression is a constant expression.















  return CheckConstantExpression(Info, E->getExprLoc(), E->getType(), Result) &&










@@ -13637,25 +13625,15 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,



























  if (Info.EnableNewConstInterp) {















    auto &InterpCtx = const_cast<ASTContext &>(Ctx).getInterpContext();













    switch (InterpCtx.evaluateAsInitializer(Info, VD, Value)) {













    case interp::InterpResult::Fail:













      // Bail out if an error was encountered.













      return false;













    case interp::InterpResult::Success:













      // Evaluation succeeded and value was set.













      return CheckConstantExpression(Info, DeclLoc, DeclTy, Value);













    case interp::InterpResult::Bail:













      // Evaluate the value again for the tree evaluator to use.













      break;













    }













  }

























    if (!InterpCtx.evaluateAsInitializer(Info, VD, Value))













      return false;













  } else {















    LValue LVal;















    LVal.set(VD);





























    // C++11 [basic.start.init]p2:













  //  Variables with static storage duration or thread storage duration shall be













  //  zero-initialized before any other initialization takes place.













    //  Variables with static storage duration or thread storage duration shall













    //  be zero-initialized before any other initialization takes place.















    // This behavior is not present in C.















    if (Ctx.getLangOpts().CPlusPlus && !VD->hasLocalStorage() &&















        !DeclTy->isReferenceType()) {









@@ -13676,7 +13654,7 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,



























    if (!Info.discardCleanups())















      llvm_unreachable("Unhandled cleanup; missing full expression marker?");

























  }















  return CheckConstantExpression(Info, DeclLoc, DeclTy, Value) &&















         CheckMemoryLeaks(Info);















}










@@ -14415,14 +14393,8 @@ bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,



























  // The constexpr VM attempts to compile all methods to bytecode here.















  if (Info.EnableNewConstInterp) {













    auto &InterpCtx = Info.Ctx.getInterpContext();













    switch (InterpCtx.isPotentialConstantExpr(Info, FD)) {













    case interp::InterpResult::Success:













    case interp::InterpResult::Fail:













    Info.Ctx.getInterpContext().isPotentialConstantExpr(Info, FD);















    return Diags.empty();













    case interp::InterpResult::Bail:













      break;













    }















  }





























  const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);

























clang/lib/AST/Interp/Context.cpp



+22
−42






























Original line number
Diff line number
Diff line








@@ -21,43 +21,36 @@













using namespace clang;















using namespace clang::interp;





























Context::Context(ASTContext &Ctx)













    : Ctx(Ctx), ForceInterp(getLangOpts().ForceNewConstInterp),













      P(new Program(*this)) {}













Context::Context(ASTContext &Ctx) : Ctx(Ctx), P(new Program(*this)) {}































Context::~Context() {}





























InterpResult Context::isPotentialConstantExpr(State &Parent,













                                              const FunctionDecl *FD) {













bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {















  Function *Func = P->getFunction(FD);















  if (!Func) {















    if (auto R = ByteCodeStmtGen<ByteCodeEmitter>(*this, *P).compileFunc(FD)) {















      Func = *R;













    } else if (ForceInterp) {













    } else {















      handleAllErrors(R.takeError(), [&Parent](ByteCodeGenError &Err) {















        Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);















      });













      return InterpResult::Fail;













    } else {













      consumeError(R.takeError());













      return InterpResult::Bail;













      return false;















    }















  }































  if (!Func->isConstexpr())













    return InterpResult::Fail;













    return false;































  APValue Dummy;















  return Run(Parent, Func, Dummy);















}





























InterpResult Context::evaluateAsRValue(State &Parent, const Expr *E,













                                       APValue &Result) {













bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {















  ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);















  return Check(Parent, C.interpretExpr(E));















}





























InterpResult Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,













bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,















                                    APValue &Result) {















  ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);















  return Check(Parent, C.interpretDecl(VD));










@@ -116,33 +109,20 @@ unsigned Context::getCharBit() const {













  return Ctx.getTargetInfo().getCharWidth();















}





























InterpResult Context::Run(State &Parent, Function *Func, APValue &Result) {













  InterpResult Flag;













  {













bool Context::Run(State &Parent, Function *Func, APValue &Result) {















  InterpState State(Parent, *P, Stk, *this);















  State.Current = new InterpFrame(State, Func, nullptr, {}, {});













    if (Interpret(State, Result)) {













      Flag = InterpResult::Success;













    } else {













      Flag = InterpResult::Fail;













    }













  }



























  if (Flag != InterpResult::Success)













  if (Interpret(State, Result))













    return true;















  Stk.clear();













  return Flag;













  return false;















}





























InterpResult Context::Check(State &Parent, llvm::Expected<bool> &&R) {













  if (R) {













    return *R ? InterpResult::Success : InterpResult::Fail;













  } else if (ForceInterp) {













    handleAllErrors(R.takeError(), [&Parent](ByteCodeGenError &Err) {













bool Context::Check(State &Parent, llvm::Expected<bool> &&Flag) {













  if (Flag)













    return *Flag;













  handleAllErrors(Flag.takeError(), [&Parent](ByteCodeGenError &Err) {















    Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);















  });













    return InterpResult::Fail;













  } else {













    consumeError(R.takeError());













    return InterpResult::Bail;













  }













  return false;















}