[cfe-commits] r103867 - in /cfe/trunk: lib/CodeGen/CGDecl.cpp lib/CodeGen/CGStmt.cpp lib/Sema/SemaStmt.cpp test/CodeGenCXX/nrvo.cpp

[Douglas Gregor]

On May 14, 2010, at 11:46 PM, Douglas Gregor wrote:

> Author: dgregor
> Date: Sat May 15 01:46:45 2010
> New Revision: 103867
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=103867&view=rev
> Log:
> Implement a simple form of the C++ named return value optimization for
> return statements. We perform NRVO only when all of the return
> statements in the function return the same variable. Fixes some link
> failures in Boost.Interprocess (which is relying on NRVO), and
> probably improves performance for some C++ applications.
> 
> Added:
>    cfe/trunk/test/CodeGenCXX/nrvo.cpp   (with props)
> Modified:
>    cfe/trunk/lib/CodeGen/CGDecl.cpp
>    cfe/trunk/lib/CodeGen/CGStmt.cpp
>    cfe/trunk/lib/Sema/SemaStmt.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=103867&r1=103866&r2=103867&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Sat May 15 01:46:45 2010
> @@ -398,16 +398,19 @@
>   CharUnits Align = CharUnits::Zero();
>   bool IsSimpleConstantInitializer = false;
> 
> +  bool NRVO = false;
>   llvm::Value *DeclPtr;
>   if (Ty->isConstantSizeType()) {
>     if (!Target.useGlobalsForAutomaticVariables()) {
> -      
> +      NRVO = getContext().getLangOptions().ElideConstructors && 
> +             D.isNRVOVariable();
>       // If this value is an array or struct, is POD, and if the initializer is
> -      // a staticly determinable constant, try to optimize it.
> +      // a staticly determinable constant, try to optimize it (unless the NRVO
> +      // is already optimizing this).
>       if (D.getInit() && !isByRef &&
>           (Ty->isArrayType() || Ty->isRecordType()) &&
>           Ty->isPODType() &&
> -          D.getInit()->isConstantInitializer(getContext())) {
> +          D.getInit()->isConstantInitializer(getContext()) && !NRVO) {
>         // If this variable is marked 'const', emit the value as a global.
>         if (CGM.getCodeGenOpts().MergeAllConstants &&
>             Ty.isConstant(getContext())) {
> @@ -418,19 +421,29 @@
>         IsSimpleConstantInitializer = true;
>       }
> 
> -      // A normal fixed sized variable becomes an alloca in the entry block.
> +      // A normal fixed sized variable becomes an alloca in the entry block,
> +      // unless it's an NRVO variable.
>       const llvm::Type *LTy = ConvertTypeForMem(Ty);
> -      if (isByRef)
> -        LTy = BuildByRefType(&D);
> -      llvm::AllocaInst *Alloc = CreateTempAlloca(LTy);
> -      Alloc->setName(D.getNameAsString());
> -
> -      Align = getContext().getDeclAlign(&D);
> -      if (isByRef)
> -        Align = std::max(Align, 
> -            CharUnits::fromQuantity(Target.getPointerAlign(0) / 8));
> -      Alloc->setAlignment(Align.getQuantity());
> -      DeclPtr = Alloc;
> +      
> +      if (NRVO) {
> +        // The named return value optimization: allocate this variable in the
> +        // return slot, so that we can elide the copy when returning this
> +        // variable (C++0x [class.copy]p34).
> +        DeclPtr = ReturnValue;
> +      } else {
> +        if (isByRef)
> +          LTy = BuildByRefType(&D);
> +        
> +        llvm::AllocaInst *Alloc = CreateTempAlloca(LTy);
> +        Alloc->setName(D.getNameAsString());
> +
> +        Align = getContext().getDeclAlign(&D);
> +        if (isByRef)
> +          Align = std::max(Align, 
> +              CharUnits::fromQuantity(Target.getPointerAlign(0) / 8));
> +        Alloc->setAlignment(Align.getQuantity());
> +        DeclPtr = Alloc;
> +      }
>     } else {
>       // Targets that don't support recursion emit locals as globals.
>       const char *Class =
> @@ -645,13 +658,14 @@
>   while (const ArrayType *Array = getContext().getAsArrayType(DtorTy))
>     DtorTy = getContext().getBaseElementType(Array);
>   if (const RecordType *RT = DtorTy->getAs<RecordType>())
> -    if (CXXRecordDecl *ClassDecl = dyn_cast<CXXRecordDecl>(RT->getDecl())) {
> -      llvm::Value *Loc = DeclPtr;
> -      if (isByRef)
> -        Loc = Builder.CreateStructGEP(DeclPtr, getByRefValueLLVMField(&D), 
> -                                      D.getNameAsString());
> -      
> -      if (!ClassDecl->hasTrivialDestructor()) {
> +    if (CXXRecordDecl *ClassDecl = dyn_cast<CXXRecordDecl>(RT->getDecl())) {      
> +      if (!ClassDecl->hasTrivialDestructor() && !NRVO) {
> +        // Note: We suppress the destructor call when this is an NRVO variable.
> +        llvm::Value *Loc = DeclPtr;
> +        if (isByRef)
> +          Loc = Builder.CreateStructGEP(DeclPtr, getByRefValueLLVMField(&D), 
> +                                        D.getNameAsString());
> +        
>         const CXXDestructorDecl *D = ClassDecl->getDestructor(getContext());
>         assert(D && "EmitLocalBlockVarDecl - destructor is nul");

And... this is broken. We need to destroy the local variable if an exception is thrown, but not if we return. Investigating a fix.

	- Doug