[clang][bytecode] Simplify and optimize EvaluationResult

[tbaederr]

Remove support for saving a Pointer in the EvaluationResult since that was unused. Add stealAPValue() which moves the APValue out of the EvaluationResult to avoid a copy at the end of the evaluation.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Remove support for saving a Pointer in the EvaluationResult since that was unused. Add stealAPValue() which moves the APValue out of the EvaluationResult to avoid a copy at the end of the evaluation.

---

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

4 Files Affected:

• (modified) clang/lib/AST/ByteCode/Context.cpp (+3-3)
• (modified) clang/lib/AST/ByteCode/Disasm.cpp (+5-29)
• (modified) clang/lib/AST/ByteCode/EvaluationResult.cpp (+1-31)
• (modified) clang/lib/AST/ByteCode/EvaluationResult.h (+13-18)

diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index fbbb508ed226c..8598996681466 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -91,7 +91,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
 #endif
   }
 
-  Result = Res.toAPValue();
+  Result = Res.stealAPValue();
 
   return true;
 }
@@ -121,7 +121,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result,
 #endif
   }
 
-  Result = Res.toAPValue();
+  Result = Res.stealAPValue();
   return true;
 }
 
@@ -153,7 +153,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
 #endif
   }
 
-  Result = Res.toAPValue();
+  Result = Res.stealAPValue();
   return true;
 }
 
diff --git a/clang/lib/AST/ByteCode/Disasm.cpp b/clang/lib/AST/ByteCode/Disasm.cpp
index 4ea47eac2150c..6ca6e93c46177 100644
--- a/clang/lib/AST/ByteCode/Disasm.cpp
+++ b/clang/lib/AST/ByteCode/Disasm.cpp
@@ -551,37 +551,13 @@ LLVM_DUMP_METHOD void Block::dump(llvm::raw_ostream &OS) const {
 LLVM_DUMP_METHOD void EvaluationResult::dump() const {
   assert(Ctx);
   auto &OS = llvm::errs();
-  const ASTContext &ASTCtx = Ctx->getASTContext();
 
-  switch (Kind) {
-  case Empty:
+  if (empty()) {
     OS << "Empty\n";
-    break;
-  case RValue:
-    OS << "RValue: ";
-    std::get<APValue>(Value).dump(OS, ASTCtx);
-    break;
-  case LValue: {
-    assert(Source);
-    QualType SourceType;
-    if (const auto *D = dyn_cast<const Decl *>(Source)) {
-      if (const auto *VD = dyn_cast<ValueDecl>(D))
-        SourceType = VD->getType();
-    } else if (const auto *E = dyn_cast<const Expr *>(Source)) {
-      SourceType = E->getType();
-    }
-
-    OS << "LValue: ";
-    if (const auto *P = std::get_if<Pointer>(&Value))
-      P->toAPValue(ASTCtx).printPretty(OS, ASTCtx, SourceType);
-    OS << "\n";
-    break;
-  }
-  case Invalid:
+  } else if (isInvalid()) {
     OS << "Invalid\n";
-    break;
-  case Valid:
-    OS << "Valid\n";
-    break;
+  } else {
+    OS << "Value: ";
+    Value.dump(OS, Ctx->getASTContext());
   }
 }
diff --git a/clang/lib/AST/ByteCode/EvaluationResult.cpp b/clang/lib/AST/ByteCode/EvaluationResult.cpp
index de049b4da90a7..19a7eeb8ce240 100644
--- a/clang/lib/AST/ByteCode/EvaluationResult.cpp
+++ b/clang/lib/AST/ByteCode/EvaluationResult.cpp
@@ -8,6 +8,7 @@
 
 #include "EvaluationResult.h"
 #include "InterpState.h"
+#include "Pointer.h"
 #include "Record.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
@@ -16,37 +17,6 @@
 namespace clang {
 namespace interp {
 
-APValue EvaluationResult::toAPValue() const {
-  assert(!empty());
-  switch (Kind) {
-  case LValue:
-    // Either a pointer or a function pointer.
-    if (const auto *P = std::get_if<Pointer>(&Value))
-      return P->toAPValue(Ctx->getASTContext());
-    else
-      llvm_unreachable("Unhandled LValue type");
-    break;
-  case RValue:
-    return std::get<APValue>(Value);
-  case Valid:
-    return APValue();
-  default:
-    llvm_unreachable("Unhandled result kind?");
-  }
-}
-
-std::optional<APValue> EvaluationResult::toRValue() const {
-  if (Kind == RValue)
-    return toAPValue();
-
-  assert(Kind == LValue);
-
-  // We have a pointer and want an RValue.
-  if (const auto *P = std::get_if<Pointer>(&Value))
-    return P->toRValue(*Ctx, getSourceType());
-  llvm_unreachable("Unhandled lvalue kind");
-}
-
 static void DiagnoseUninitializedSubobject(InterpState &S, SourceLocation Loc,
                                            const FieldDecl *SubObjDecl) {
   assert(SubObjDecl && "Subobject declaration does not exist");
diff --git a/clang/lib/AST/ByteCode/EvaluationResult.h b/clang/lib/AST/ByteCode/EvaluationResult.h
index 9785ec564d76e..d00b9ca02a48d 100644
--- a/clang/lib/AST/ByteCode/EvaluationResult.h
+++ b/clang/lib/AST/ByteCode/EvaluationResult.h
@@ -9,7 +9,6 @@
 #ifndef LLVM_CLANG_AST_INTERP_EVALUATION_RESULT_H
 #define LLVM_CLANG_AST_INTERP_EVALUATION_RESULT_H
 
-#include "Pointer.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
@@ -20,6 +19,9 @@ namespace clang {
 namespace interp {
 class EvalEmitter;
 class Context;
+class Pointer;
+class SourceInfo;
+class InterpState;
 
 /// Defines the result of an evaluation.
 ///
@@ -32,8 +34,6 @@ class EvaluationResult final {
 public:
   enum ResultKind {
     Empty,   // Initial state.
-    LValue,  // Result is an lvalue/pointer.
-    RValue,  // Result is an rvalue.
     Invalid, // Result is invalid.
     Valid,   // Result is valid and empty.
   };
@@ -42,7 +42,7 @@ class EvaluationResult final {
 
 private:
   const Context *Ctx = nullptr;
-  std::variant<std::monostate, Pointer, APValue> Value;
+  APValue Value;
   ResultKind Kind = Empty;
   DeclTy Source = nullptr; // Currently only needed for dump().
 
@@ -55,10 +55,8 @@ class EvaluationResult final {
   void setSource(DeclTy D) { Source = D; }
 
   void takeValue(APValue &&V) {
-    // V could still be an LValue.
     assert(empty());
     Value = std::move(V);
-    Kind = RValue;
   }
   void setInvalid() {
     // We are NOT asserting empty() here, since setting it to invalid
@@ -75,18 +73,15 @@ class EvaluationResult final {
 
   bool empty() const { return Kind == Empty; }
   bool isInvalid() const { return Kind == Invalid; }
-  bool isLValue() const { return Kind == LValue; }
-  bool isRValue() const { return Kind == RValue; }
-  bool isPointer() const { return std::holds_alternative<Pointer>(Value); }
-
-  /// Returns an APValue for the evaluation result. The returned
-  /// APValue might be an LValue or RValue.
-  APValue toAPValue() const;
-
-  /// If the result is an LValue, convert that to an RValue
-  /// and return it. This may fail, e.g. if the result is an
-  /// LValue and we can't read from it.
-  std::optional<APValue> toRValue() const;
+
+  /// Returns an APValue for the evaluation result.
+  APValue toAPValue() const {
+    assert(!empty());
+    assert(!isInvalid());
+    return Value;
+  }
+
+  APValue stealAPValue() { return std::move(Value); }
 
   /// Check that all subobjects of the given pointer have been initialized.
   bool checkFullyInitialized(InterpState &S, const Pointer &Ptr) const;

[tbaederr]

FYI: https://llvm-compile-time-tracker.com/compare.php?from=c295ad9867e8492c27ca3f906f7bbc9a02360e72&to=3c23ae99f2f88d2a1c0f35b82a356f42e692e335&stat=instructions:u