[clang] [clang][bytecode][NFC] Remove APValue Result argument where unnecessary (PR #118199)

via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 1 00:57:25 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

This is unneeded in almost all circumstances. We only return an APValue back to clang when the evaluation is finished, and that is always done by an EvalEmitter - which has its own implementation of the Ret instructions.

---
Full diff: https://github.com/llvm/llvm-project/pull/118199.diff


6 Files Affected:

- (modified) clang/lib/AST/ByteCode/Context.cpp (+3-4) 
- (modified) clang/lib/AST/ByteCode/Context.h (+1-1) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+4-6) 
- (modified) clang/lib/AST/ByteCode/Interp.h (+5-12) 
- (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+4-5) 
- (modified) clang/utils/TableGen/ClangOpcodesEmitter.cpp (-2) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index 7088cf02901c63..b52892cf69bfb6 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -34,8 +34,7 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
   if (!Func)
     return false;
 
-  APValue DummyResult;
-  if (!Run(Parent, Func, DummyResult))
+  if (!Run(Parent, Func))
     return false;
 
   return Func->isConstexpr();
@@ -213,13 +212,13 @@ const llvm::fltSemantics &Context::getFloatSemantics(QualType T) const {
   return Ctx.getFloatTypeSemantics(T);
 }
 
-bool Context::Run(State &Parent, const Function *Func, APValue &Result) {
+bool Context::Run(State &Parent, const Function *Func) {
 
   {
     InterpState State(Parent, *P, Stk, *this);
     State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, CodePtr(),
                                     Func->getArgSize());
-    if (Interpret(State, Result)) {
+    if (Interpret(State)) {
       assert(Stk.empty());
       return true;
     }
diff --git a/clang/lib/AST/ByteCode/Context.h b/clang/lib/AST/ByteCode/Context.h
index e0d4bafdebaf2b..ed539def99efd0 100644
--- a/clang/lib/AST/ByteCode/Context.h
+++ b/clang/lib/AST/ByteCode/Context.h
@@ -114,7 +114,7 @@ class Context final {
 
 private:
   /// Runs a function.
-  bool Run(State &Parent, const Function *Func, APValue &Result);
+  bool Run(State &Parent, const Function *Func);
 
   /// Current compilation context.
   ASTContext &Ctx;
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 329f1584be34bf..435af1201890c8 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -27,7 +27,7 @@
 using namespace clang;
 using namespace clang::interp;
 
-static bool RetValue(InterpState &S, CodePtr &Pt, APValue &Result) {
+static bool RetValue(InterpState &S, CodePtr &Pt) {
   llvm::report_fatal_error("Interpreter cannot return values");
 }
 
@@ -1205,11 +1205,10 @@ bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
   InterpFrame *FrameBefore = S.Current;
   S.Current = NewFrame.get();
 
-  APValue CallResult;
   // Note that we cannot assert(CallResult.hasValue()) here since
   // Ret() above only sets the APValue if the curent frame doesn't
   // have a caller set.
-  if (Interpret(S, CallResult)) {
+  if (Interpret(S)) {
     NewFrame.release(); // Frame was delete'd already.
     assert(S.Current == FrameBefore);
     return true;
@@ -1270,11 +1269,10 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
   S.Current = NewFrame.get();
 
   InterpStateCCOverride CCOverride(S, Func->getDecl()->isImmediateFunction());
-  APValue CallResult;
   // Note that we cannot assert(CallResult.hasValue()) here since
   // Ret() above only sets the APValue if the curent frame doesn't
   // have a caller set.
-  if (Interpret(S, CallResult)) {
+  if (Interpret(S)) {
     NewFrame.release(); // Frame was delete'd already.
     assert(S.Current == FrameBefore);
     return true;
@@ -1598,7 +1596,7 @@ bool CheckBitCast(InterpState &S, CodePtr OpPC, bool HasIndeterminateBits,
 #if defined(_MSC_VER) && !defined(__clang__) && !defined(NDEBUG)
 #pragma optimize("", off)
 #endif
-bool Interpret(InterpState &S, APValue &Result) {
+bool Interpret(InterpState &S) {
   // The current stack frame when we started Interpret().
   // This is being used by the ops to determine wheter
   // to return from this function and thus terminate
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 47dcfca79f7356..25740c90d240ab 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -41,13 +41,6 @@ namespace interp {
 using APSInt = llvm::APSInt;
 using FixedPointSemantics = llvm::FixedPointSemantics;
 
-/// Convert a value to an APValue.
-template <typename T>
-bool ReturnValue(const InterpState &S, const T &V, APValue &R) {
-  R = V.toAPValue(S.getASTContext());
-  return true;
-}
-
 /// Checks if the variable has externally defined storage.
 bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
 
@@ -299,7 +292,7 @@ bool CheckFloatResult(InterpState &S, CodePtr OpPC, const Floating &Result,
 bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR);
 
 /// Interpreter entry point.
-bool Interpret(InterpState &S, APValue &Result);
+bool Interpret(InterpState &S);
 
 /// Interpret a builtin function.
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
@@ -321,7 +314,7 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC,
                               const Function *Func);
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
-bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
+bool Ret(InterpState &S, CodePtr &PC) {
   const T &Ret = S.Stk.pop<T>();
 
   // Make sure returned pointers are live. We might be trying to return a
@@ -349,13 +342,13 @@ bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   } else {
     delete S.Current;
     S.Current = nullptr;
-    if (!ReturnValue<T>(S, Ret, Result))
-      return false;
+    // The topmost frame should come from an EvalEmitter,
+    // which has its own implementation of the Ret<> instruction.
   }
   return true;
 }
 
-inline bool RetVoid(InterpState &S, CodePtr &PC, APValue &Result) {
+inline bool RetVoid(InterpState &S, CodePtr &PC) {
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
 
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index b450d8263c30bf..817adb3f48eb87 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -116,14 +116,14 @@ static void assignInteger(Pointer &Dest, PrimType ValueT, const APSInt &Value) {
       ValueT, { Dest.deref<T>() = T::from(static_cast<T>(Value)); });
 }
 
-static bool retPrimValue(InterpState &S, CodePtr OpPC, APValue &Result,
+static bool retPrimValue(InterpState &S, CodePtr OpPC,
                          std::optional<PrimType> &T) {
   if (!T)
-    return RetVoid(S, OpPC, Result);
+    return RetVoid(S, OpPC);
 
 #define RET_CASE(X)                                                            \
   case X:                                                                      \
-    return Ret<X>(S, OpPC, Result);
+    return Ret<X>(S, OpPC);
   switch (*T) {
     RET_CASE(PT_Ptr);
     RET_CASE(PT_FnPtr);
@@ -1687,7 +1687,6 @@ static bool interp__builtin_arithmetic_fence(InterpState &S, CodePtr OpPC,
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
                       const CallExpr *Call, uint32_t BuiltinID) {
   const InterpFrame *Frame = S.Current;
-  APValue Dummy;
 
   std::optional<PrimType> ReturnT = S.getContext().classify(Call);
 
@@ -2138,7 +2137,7 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
     return false;
   }
 
-  return retPrimValue(S, OpPC, Dummy, ReturnT);
+  return retPrimValue(S, OpPC, ReturnT);
 }
 
 bool InterpretOffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E,
diff --git a/clang/utils/TableGen/ClangOpcodesEmitter.cpp b/clang/utils/TableGen/ClangOpcodesEmitter.cpp
index d6ab307dd80bcf..64534a50877eca 100644
--- a/clang/utils/TableGen/ClangOpcodesEmitter.cpp
+++ b/clang/utils/TableGen/ClangOpcodesEmitter.cpp
@@ -146,8 +146,6 @@ void ClangOpcodesEmitter::EmitInterp(raw_ostream &OS, StringRef N,
                 OS << ", PC";
               else
                 OS << ", OpPC";
-              if (CanReturn)
-                OS << ", Result";
               for (size_t I = 0, N = Args.size(); I < N; ++I)
                 OS << ", V" << I;
               OS << "))\n";

``````````

</details>


https://github.com/llvm/llvm-project/pull/118199


More information about the cfe-commits mailing list