[clang] 7c215a4 - [clang][Interp] Explicitly handle RVO Pointer

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-11-30T10:37:57+01:00
New Revision: 7c215a457178174187cb975e6ee7d96105ea0ef8

URL: https://github.com/llvm/llvm-project/commit/7c215a457178174187cb975e6ee7d96105ea0ef8
DIFF: https://github.com/llvm/llvm-project/commit/7c215a457178174187cb975e6ee7d96105ea0ef8.diff

LOG: [clang][Interp] Explicitly handle RVO Pointer

The calling convention is:

[RVO pointer]
[instance pointer]
[... args ...]

We handle the instance pointer ourselves, BUT for the RVO pointer, we
just assumed in visitReturnStmt() that it is on top of the stack. Which
isn't true if there are other args present (and a this pointer, maybe).

Fix this by recording the RVO pointer explicitly when creating an
InterpFrame, just like we do with the instance/This pointer.

There is already a "RVOAndParams()" test in test/AST/Inter/records.cpp,
that was supposed to test this, however, it didn't trigger any
problematic behavior because the parameter and the return value have the
same type.

Differential Revision: https://reviews.llvm.org/D137392

Added: 
    

Modified: 
    clang/lib/AST/Interp/ByteCodeStmtGen.cpp
    clang/lib/AST/Interp/Interp.h
    clang/lib/AST/Interp/InterpFrame.cpp
    clang/lib/AST/Interp/InterpFrame.h
    clang/lib/AST/Interp/Opcodes.td
    clang/test/AST/Interp/records.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 19cdc2a00888..69b223299313 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -233,8 +233,13 @@ bool ByteCodeStmtGen<Emitter>::visitReturnStmt(const ReturnStmt *RS) {
       return this->emitRet(*ReturnType, RS);
     } else {
       // RVO - construct the value in the return location.
+      if (!this->emitRVOPtr(RE))
+        return false;
       if (!this->visitInitializer(RE))
         return false;
+      if (!this->emitPopPtr(RE))
+        return false;
+
       this->emitCleanup();
       return this->emitRetVoid(RS);
     }

diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 5a6c3f16f8a7..e252c54c8873 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1148,6 +1148,12 @@ inline bool This(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+inline bool RVOPtr(InterpState &S, CodePtr OpPC) {
+  assert(S.Current->getFunction()->hasRVO());
+  S.Stk.push<Pointer>(S.Current->getRVOPtr());
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Shr, Shl
 //===----------------------------------------------------------------------===//

diff  --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp
index 781b6d8a845a..9bdb71237b5d 100644
--- a/clang/lib/AST/Interp/InterpFrame.cpp
+++ b/clang/lib/AST/Interp/InterpFrame.cpp
@@ -49,6 +49,9 @@ 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));

diff  --git a/clang/lib/AST/Interp/InterpFrame.h b/clang/lib/AST/Interp/InterpFrame.h
index ecbe697eacdd..bcb45a1b9174 100644
--- a/clang/lib/AST/Interp/InterpFrame.h
+++ b/clang/lib/AST/Interp/InterpFrame.h
@@ -37,7 +37,8 @@ class InterpFrame final : public Frame {
 
   /// Creates a new frame with the values that make sense.
   /// I.e., the caller is the current frame of S,
-  /// and the This() pointer is the current Pointer on the top of S's stack,
+  /// the This() pointer is the current Pointer on the top of S's stack,
+  /// and the RVO pointer is before that.
   InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC);
 
   /// Destroys the frame, killing all live pointers to stack slots.
@@ -101,6 +102,9 @@ class InterpFrame final : public Frame {
   /// Returns the 'this' pointer.
   const Pointer &getThis() const { return This; }
 
+  /// Returns the RVO pointer, if the Function has one.
+  const Pointer &getRVOPtr() const { return RVOPtr; }
+
   /// Checks if the frame is a root frame - return should quit the interpreter.
   bool isRoot() const { return !Func; }
 
@@ -139,6 +143,8 @@ class InterpFrame final : public Frame {
   const Function *Func;
   /// Current object pointer for methods.
   Pointer This;
+  /// Pointer the non-primitive return value gets constructed in.
+  Pointer RVOPtr;
   /// Return address.
   CodePtr RetPC;
   /// The size of all the arguments.

diff  --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index ebb0f49bfe59..058475b2d399 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -269,6 +269,9 @@ def GetPtrThisVirtBase : Opcode {
 // [] -> [Pointer]
 def This : Opcode;
 
+// [] -> [Pointer]
+def RVOPtr : Opcode;
+
 // [Pointer] -> [Pointer]
 def NarrowPtr : Opcode;
 // [Pointer] -> [Pointer]

diff  --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 963a3f1d636f..d556ad064bd6 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -131,6 +131,12 @@ constexpr C RVOAndParams(const C *c) {
   return C();
 }
 constexpr C RVOAndParamsResult = RVOAndParams(&c);
+
+/// Parameter and return value have 
diff erent types.
+constexpr C RVOAndParams(int a) {
+  return C();
+}
+constexpr C RVOAndParamsResult2 = RVOAndParams(12);
 #endif
 
 class Bar { // expected-note {{definition of 'Bar' is not complete}} \