tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137392
Files:
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
Index: clang/test/AST/Interp/records.cpp
===================================================================
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -131,6 +131,12 @@
return C();
}
constexpr C RVOAndParamsResult = RVOAndParams(&c);
+
+/// Parameter and return value have different types.
+constexpr C RVOAndParams(int a) {
+ return C();
+}
+constexpr C RVOAndParamsResult2 = RVOAndParams(12);
#endif
class Bar { // expected-note {{definition of 'Bar' is not complete}} \
Index: clang/lib/AST/Interp/Opcodes.td
===================================================================
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -287,6 +287,9 @@
// [] -> [Pointer]
def This : Opcode;
+// [] -> [Pointer]
+def RVOPtr : Opcode;
+
// [Pointer] -> [Pointer]
def NarrowPtr : Opcode;
// [Pointer] -> [Pointer]
Index: clang/lib/AST/Interp/InterpFrame.h
===================================================================
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -37,7 +37,8 @@
/// 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.
@@ -102,6 +103,9 @@
/// 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; }
@@ -145,6 +149,8 @@
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.
Index: clang/lib/AST/Interp/InterpFrame.cpp
===================================================================
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -52,6 +52,9 @@
// 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));
Index: clang/lib/AST/Interp/Interp.h
===================================================================
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1265,6 +1265,12 @@
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
//===----------------------------------------------------------------------===//
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===================================================================
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -233,8 +233,13 @@
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
