[clang][bytecode] Use stack offsets for This/RVO ptrs

[tbaederr]

Instead of keeping the Pointers itself in InterpFrame, just save them as offsets and use stackRef<>() when we need them.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Instead of keeping the Pointers itself in InterpFrame, just save them as offsets and use stackRef<>() when we need them.

---

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

4 Files Affected:

• (modified) clang/lib/AST/ByteCode/Interp.cpp (+4-4)
• (modified) clang/lib/AST/ByteCode/Interp.h (+25-25)
• (modified) clang/lib/AST/ByteCode/InterpFrame.cpp (+6-9)
• (modified) clang/lib/AST/ByteCode/InterpFrame.h (+12-6)

diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 0f322f6ed42ac..87306fdc8f842 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1027,8 +1027,8 @@ static bool CheckCallDepth(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
-bool CheckThis(InterpState &S, CodePtr OpPC, const Pointer &This) {
-  if (!This.isZero())
+bool CheckThis(InterpState &S, CodePtr OpPC) {
+  if (S.Current->hasThisPointer())
     return true;
 
   const Expr *E = S.Current->getExpr(OpPC);
@@ -1198,8 +1198,8 @@ static bool runRecordDestructor(InterpState &S, CodePtr OpPC,
   const Record *R = Desc->ElemRecord;
   assert(R);
 
-  if (Pointer::pointToSameBlock(BasePtr, S.Current->getThis()) &&
-      S.Current->getFunction()->isDestructor()) {
+  if (S.Current->hasThisPointer() &&S.Current->getFunction()->isDestructor() &&
+      Pointer::pointToSameBlock(BasePtr, S.Current->getThis()) {
     const SourceInfo &Loc = S.Current->getSource(OpPC);
     S.FFDiag(Loc, diag::note_constexpr_double_destroy);
     return false;
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index b3b4b998439cc..3bc1a67feeba2 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -104,7 +104,7 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
 bool CheckInit(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
 
 /// Checks the 'this' pointer.
-bool CheckThis(InterpState &S, CodePtr OpPC, const Pointer &This);
+bool CheckThis(InterpState &S, CodePtr OpPC);
 
 /// Checks if dynamic memory allocation is available in the current
 /// language mode.
@@ -1440,9 +1440,9 @@ template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool GetThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
   if (S.checkingPotentialConstantExpression())
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   const Pointer &Field = This.atField(I);
   if (!CheckLoad(S, OpPC, Field))
     return false;
@@ -1454,10 +1454,10 @@ template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool SetThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
   if (S.checkingPotentialConstantExpression())
     return false;
+  if (!CheckThis(S, OpPC))
+    return false;
   const T &Value = S.Stk.pop<T>();
   const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
-    return false;
   const Pointer &Field = This.atField(I);
   if (!CheckStore(S, OpPC, Field))
     return false;
@@ -1560,9 +1560,9 @@ template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
   if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0)
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   const Pointer &Field = This.atField(I);
   assert(Field.canBeInitialized());
   Field.deref<T>() = S.Stk.pop<T>();
@@ -1574,9 +1574,9 @@ template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitThisFieldActivate(InterpState &S, CodePtr OpPC, uint32_t I) {
   if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0)
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   const Pointer &Field = This.atField(I);
   assert(Field.canBeInitialized());
   Field.deref<T>() = S.Stk.pop<T>();
@@ -1593,9 +1593,9 @@ bool InitThisBitField(InterpState &S, CodePtr OpPC, const Record::Field *F,
   assert(F->isBitField());
   if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0)
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   const Pointer &Field = This.atField(FieldOffset);
   assert(Field.canBeInitialized());
   const auto &Value = S.Stk.pop<T>();
@@ -1610,9 +1610,9 @@ bool InitThisBitFieldActivate(InterpState &S, CodePtr OpPC,
   assert(F->isBitField());
   if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0)
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   const Pointer &Field = This.atField(FieldOffset);
   assert(Field.canBeInitialized());
   const auto &Value = S.Stk.pop<T>();
@@ -1750,9 +1750,9 @@ bool GetPtrFieldPop(InterpState &S, CodePtr OpPC, uint32_t Off);
 inline bool GetPtrThisField(InterpState &S, CodePtr OpPC, uint32_t Off) {
   if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0)
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   S.Stk.push<Pointer>(This.atField(Off));
   return true;
 }
@@ -1844,9 +1844,9 @@ inline bool GetMemberPtrBasePop(InterpState &S, CodePtr OpPC, int32_t Off) {
 inline bool GetPtrThisBase(InterpState &S, CodePtr OpPC, uint32_t Off) {
   if (S.checkingPotentialConstantExpression())
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
   S.Stk.push<Pointer>(This.atField(Off));
   return true;
 }
@@ -1925,10 +1925,10 @@ inline bool GetPtrThisVirtBase(InterpState &S, CodePtr OpPC,
   assert(D);
   if (S.checkingPotentialConstantExpression())
     return false;
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
-  return VirtBaseHelper(S, OpPC, D, S.Current->getThis());
+  const Pointer &This = S.Current->getThis();
+  return VirtBaseHelper(S, OpPC, D, This);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1991,6 +1991,8 @@ static inline bool Activate(InterpState &S, CodePtr OpPC) {
 static inline bool ActivateThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
   if (S.checkingPotentialConstantExpression())
     return false;
+  if (!S.Current->hasThisPointer())
+    return false;
 
   const Pointer &Ptr = S.Current->getThis();
   assert(Ptr.atField(I).canBeInitialized());
@@ -2813,13 +2815,11 @@ inline bool IsNonNull(InterpState &S, CodePtr OpPC) {
 
 inline bool This(InterpState &S, CodePtr OpPC) {
   // Cannot read 'this' in this mode.
-  if (S.checkingPotentialConstantExpression()) {
+  if (S.checkingPotentialConstantExpression())
     return false;
-  }
-
-  const Pointer &This = S.Current->getThis();
-  if (!CheckThis(S, OpPC, This))
+  if (!CheckThis(S, OpPC))
     return false;
+  const Pointer &This = S.Current->getThis();
 
   // Ensure the This pointer has been cast to the correct base.
   if (!This.isDummy()) {
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp
index c411a371282ef..bbcd7ddf538c3 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -58,15 +58,12 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC,
   // If the fuction has a This pointer, that one is next.
   // Then follow the actual arguments (but those are handled
   // in getParamPointer()).
-  if (Func->hasRVO())
-    RVOPtr = stackRef<Pointer>(0);
-
-  if (Func->hasThisPointer()) {
-    if (Func->hasRVO())
-      This = stackRef<Pointer>(sizeof(Pointer));
-    else
-      This = stackRef<Pointer>(0);
+  if (Func->hasRVO()) {
+    // RVOPtrOffset is always 0.
   }
+
+  if (Func->hasThisPointer())
+    ThisPointerOffset = Func->hasRVO() ? sizeof(Pointer) : 0;
 }
 
 InterpFrame::~InterpFrame() {
@@ -167,7 +164,7 @@ void InterpFrame::describe(llvm::raw_ostream &OS) const {
                                   /*Indentation=*/0);
       OS << ".";
     } else if (const auto *M = dyn_cast<CXXMethodDecl>(F)) {
-      print(OS, This, S.getASTContext(),
+      print(OS, getThis(), S.getASTContext(),
             S.getASTContext().getLValueReferenceType(
                 S.getASTContext().getCanonicalTagType(M->getParent())));
       OS << ".";
diff --git a/clang/lib/AST/ByteCode/InterpFrame.h b/clang/lib/AST/ByteCode/InterpFrame.h
index 129851155bd86..3cdc164e4bdda 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.h
+++ b/clang/lib/AST/ByteCode/InterpFrame.h
@@ -104,11 +104,19 @@ class InterpFrame final : public Frame {
   /// Returns a pointer to an argument - lazily creates a block.
   Pointer getParamPointer(unsigned Offset);
 
+  bool hasThisPointer() const { return Func && Func->hasThisPointer(); }
   /// Returns the 'this' pointer.
-  const Pointer &getThis() const { return This; }
+  const Pointer &getThis() const {
+    assert(hasThisPointer());
+    return stackRef<Pointer>(ThisPointerOffset);
+  }
 
   /// Returns the RVO pointer, if the Function has one.
-  const Pointer &getRVOPtr() const { return RVOPtr; }
+  const Pointer &getRVOPtr() const {
+    assert(Func);
+    assert(Func->hasRVO());
+    return stackRef<Pointer>(0);
+  }
 
   /// Checks if the frame is a root frame - return should quit the interpreter.
   bool isRoot() const { return !Func; }
@@ -163,10 +171,8 @@ class InterpFrame final : public Frame {
   unsigned Depth;
   /// Reference to the function being executed.
   const Function *Func;
-  /// Current object pointer for methods.
-  Pointer This;
-  /// Pointer the non-primitive return value gets constructed in.
-  Pointer RVOPtr;
+  /// Offset of the instance pointer. Use with stackRef<>().
+  unsigned ThisPointerOffset;
   /// Return address.
   CodePtr RetPC;
   /// The size of all the arguments.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.