tzik created this revision.
Herald added a subscriber: cfe-commits.
The previous implementation misses an opportunity to apply NRVO (Named Return
Value
Optimization) below. That discourages user to write early return code.
----------------------------------------------------------------------
struct Foo {};
Foo f(bool b) {
if (b)
return Foo();
Foo oo;
return oo;
}
-
That is, we can/should apply RVO for a return statement if it refers a
non-parameter local variable,
and the variable is referred by all return statements reachable from the
variable declaration.
While, the previous implementation disables the RVO in a scope if there are
multiple return
statements that refers different variables.
On the new algorithm, local variables are in NRVO_Candidate state at first, and
a return
statement changes it to NRVO_Disabled for all visible variables but the return
statement refers.
Then, at the end of the function AST traversal, NRVO is enabled for variables
in NRVO_Candidate
state and refers from at least one return statement.
Repository:
rC Clang
https://reviews.llvm.org/D47067
Files:
include/clang/AST/Decl.h
include/clang/Sema/Scope.h
lib/Sema/Scope.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaStmt.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
test/CodeGenCXX/nrvo.cpp
Index: test/CodeGenCXX/nrvo.cpp
===================================================================
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
}
// CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
- // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
- // CHECK: call {{.*}} @_ZN1XC1Ev
- // CHECK: call {{.*}} @_ZN1XC1ERKS_
if (B) {
X y;
return y;
}
- // FIXME: we should NRVO this variable too.
- X x;
+ // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
return x;
}
@@ -222,3 +218,16 @@
// CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
// CHECK-EH-03: attributes [[NR_NUW]] = { noreturn nounwind }
+
+
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+ if (B) {
+ // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+ return x;
+ }
+ // CHECK: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+ X y;
+ return y;
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -918,7 +918,7 @@
if (!isa<ParmVarDecl>(D)) {
Record.push_back(D->isThisDeclarationADemotedDefinition());
Record.push_back(D->isExceptionVariable());
- Record.push_back(D->isNRVOVariable());
+ Record.push_back(D->getNRVOMode());
Record.push_back(D->isCXXForRangeDecl());
Record.push_back(D->isObjCForDecl());
Record.push_back(D->isARCPseudoStrong());
@@ -2031,7 +2031,7 @@
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // InitStyle
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsThisDeclarationADemotedDefinition
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isExceptionVariable
- Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isNRVOVariable
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // NRVOMode
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isObjCForDecl
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1326,7 +1326,7 @@
VD->NonParmVarDeclBits.IsThisDeclarationADemotedDefinition =
Record.readInt();
VD->NonParmVarDeclBits.ExceptionVar = Record.readInt();
- VD->NonParmVarDeclBits.NRVOVariable = Record.readInt();
+ VD->NonParmVarDeclBits.NRVOMode = Record.readInt();
VD->NonParmVarDeclBits.CXXForRangeDecl = Record.readInt();
VD->NonParmVarDeclBits.ObjCForDecl = Record.readInt();
VD->NonParmVarDeclBits.ARCPseudoStrong = Record.readInt();
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -3455,12 +3455,9 @@
ExpressionEvaluationContext::DiscardedStatement)
return R;
- if (VarDecl *VD =
- const_cast<VarDecl*>(cast<ReturnStmt>(R.get())->getNRVOCandidate())) {
- CurScope->addNRVOCandidate(VD);
- } else {
- CurScope->setNoNRVO();
- }
+ VarDecl *VD =
+ const_cast<VarDecl*>(cast<ReturnStmt>(R.get())->getNRVOCandidate());
+ CurScope->setNRVOCandidate(VD);
CheckJumpOutOfSEHFinally(*this, ReturnLoc, *CurScope->getFnParent());
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -1798,8 +1798,6 @@
}
void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
- S->mergeNRVOIntoParent();
-
if (S->decl_empty()) return;
assert((S->getFlags() & (Scope::DeclScope | Scope::TemplateParamScope)) &&
"Scope shouldn't contain decls!");
@@ -12681,13 +12679,16 @@
/// statement in the scope of a variable has the same NRVO candidate, that
/// candidate is an NRVO variable.
void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
- ReturnStmt **Returns = Scope->Returns.data();
+ for (ReturnStmt* Return : Scope->Returns) {
+ const VarDecl* Candidate = Return->getNRVOCandidate();
+ if (!Candidate)
+ continue;
- for (unsigned I = 0, E = Scope->Returns.size(); I != E; ++I) {
- if (const VarDecl *NRVOCandidate = Returns[I]->getNRVOCandidate()) {
- if (!NRVOCandidate->isNRVOVariable())
- Returns[I]->setNRVOCandidate(nullptr);
- }
+ if (Candidate->isNRVOCandidate())
+ const_cast<VarDecl*>(Candidate)->setNRVOVariable(true);
+
+ if (!Candidate->isNRVOVariable())
+ Return->setNRVOCandidate(nullptr);
}
}
Index: lib/Sema/Scope.cpp
===================================================================
--- lib/Sema/Scope.cpp
+++ lib/Sema/Scope.cpp
@@ -92,7 +92,6 @@
UsingDirectives.clear();
Entity = nullptr;
ErrorTrap.reset();
- NRVO.setPointerAndInt(nullptr, 0);
}
bool Scope::containedInPrototypeScope() const {
@@ -119,19 +118,18 @@
Flags |= FlagsToSet;
}
-void Scope::mergeNRVOIntoParent() {
- if (VarDecl *Candidate = NRVO.getPointer()) {
- if (isDeclScope(Candidate))
- Candidate->setNRVOVariable(true);
+void Scope::setNRVOCandidate(VarDecl *Candidate) {
+ for (auto* D : DeclsInScope) {
+ VarDecl* VD = dyn_cast_or_null<VarDecl>(D);
+ if (VD && VD != Candidate && VD->isNRVOCandidate())
+ VD->setNRVOVariable(false);
}
- if (getEntity())
+ if (Candidate && isDeclScope(Candidate))
return;
- if (NRVO.getInt())
- getParent()->setNoNRVO();
- else if (NRVO.getPointer())
- getParent()->addNRVOCandidate(NRVO.getPointer());
+ if (getParent())
+ getParent()->setNRVOCandidate(Candidate);
}
LLVM_DUMP_METHOD void Scope::dump() const { dumpImpl(llvm::errs()); }
@@ -191,9 +189,4 @@
OS << "MSCurManglingNumber: " << getMSCurManglingNumber() << '\n';
if (const DeclContext *DC = getEntity())
OS << "Entity : (clang::DeclContext*)" << DC << '\n';
-
- if (NRVO.getInt())
- OS << "NRVO not allowed\n";
- else if (NRVO.getPointer())
- OS << "NRVO candidate : (clang::VarDecl*)" << NRVO.getPointer() << '\n';
}
Index: include/clang/Sema/Scope.h
===================================================================
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -201,10 +201,6 @@
/// Used to determine if errors occurred in this scope.
DiagnosticErrorTrap ErrorTrap;
- /// A lattice consisting of undefined, a single NRVO candidate variable in
- /// this scope, or over-defined. The bit is true when over-defined.
- llvm::PointerIntPair<VarDecl *, 1, bool> NRVO;
-
void setFlags(Scope *Parent, unsigned F);
public:
@@ -466,23 +462,7 @@
UsingDirectives.end());
}
- void addNRVOCandidate(VarDecl *VD) {
- if (NRVO.getInt())
- return;
- if (NRVO.getPointer() == nullptr) {
- NRVO.setPointer(VD);
- return;
- }
- if (NRVO.getPointer() != VD)
- setNoNRVO();
- }
-
- void setNoNRVO() {
- NRVO.setInt(true);
- NRVO.setPointer(nullptr);
- }
-
- void mergeNRVOIntoParent();
+ void setNRVOCandidate(VarDecl *Candidate);
/// Init - This is used by the parser to implement scope caching.
void Init(Scope *parent, unsigned flags);
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -879,6 +879,12 @@
DAK_Normal
};
+ enum NRVOMode {
+ NRVO_Candidate,
+ NRVO_Disabled,
+ NRVO_Enabled,
+ };
+
class ParmVarDeclBitfields {
friend class ASTDeclReader;
friend class ParmVarDecl;
@@ -931,7 +937,7 @@
/// Whether this local variable could be allocated in the return
/// slot of its function, enabling the named return value optimization
/// (NRVO).
- unsigned NRVOVariable : 1;
+ unsigned NRVOMode : 2;
/// Whether this variable is the for-range-declaration in a C++0x
/// for-range statement.
@@ -1319,12 +1325,20 @@
/// return slot when returning from the function. Within the function body,
/// each return that returns the NRVO object will have this variable as its
/// NRVO candidate.
+ NRVOMode getNRVOMode() const {
+ if (isa<ParmVarDecl>(this))
+ return NRVO_Disabled;
+ return static_cast<NRVOMode>(NonParmVarDeclBits.NRVOMode);
+ }
+ bool isNRVOCandidate() const {
+ return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.NRVOMode == NRVO_Candidate;
+ }
bool isNRVOVariable() const {
- return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.NRVOVariable;
+ return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.NRVOMode == NRVO_Enabled;
}
void setNRVOVariable(bool NRVO) {
assert(!isa<ParmVarDecl>(this));
- NonParmVarDeclBits.NRVOVariable = NRVO;
+ NonParmVarDeclBits.NRVOMode = NRVO ? NRVO_Enabled : NRVO_Disabled;
}
/// Determine whether this variable is the for-range-declaration in
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
