r230580 - Improvement on sized deallocation from r230160:

[John McCall]

> On Feb 25, 2015, at 3:48 PM, Larisse Voufo <lvoufo at google.com> wrote:
> Author: lvoufo
> Date: Wed Feb 25 17:48:43 2015
> New Revision: 230580
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=230580&view=rev
> Log:
> Improvement on sized deallocation from r230160:
> Do not declare sized deallocation functions dependently on whether it is found in global scope. Instead, enforce the branching in emitted code by (1) declaring the functions extern_weak and (2) emitting sized delete expressions as a branching between both forms delete.
> 
> Modified:
>    cfe/trunk/lib/CodeGen/CGExprCXX.cpp
>    cfe/trunk/lib/Sema/SemaExprCXX.cpp
>    cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
>    cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp
>    cfe/trunk/test/CodeGenCXX/pr21754.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=230580&r1=230579&r2=230580&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Wed Feb 25 17:48:43 2015
> @@ -1422,6 +1422,71 @@ CodeGenFunction::pushCallObjectDeleteCle
>                                         OperatorDelete, ElementType);
> }
> 
> +static void EmitDelete(CodeGenFunction &CGF,
> +                              const CXXDeleteExpr *DE,
> +                              llvm::Value *Ptr,
> +                              QualType ElementType);
> +
> +static void EmitSizedDelete(CodeGenFunction &CGF,
> +                            const CXXDeleteExpr *DE,
> +                            llvm::Value *Ptr,
> +                            QualType ElementType,
> +                            FunctionDecl* UnsizedDealloc) {
> +
> +  if (CGF.getLangOpts().DefineSizedDeallocation) {
> +    // The delete operator in use is fixed. So simply emit the delete expr.
> +    EmitDelete(CGF, DE, Ptr, ElementType);
> +    return;
> +  }
> +
> +  assert(UnsizedDealloc && "We must be emiting a 'sized' delete expr");
> +
> +  // Branch off over the value of operator delete:
> +  // Use the sized form if available, and default on the unsized form otherwise.
> +  llvm::BasicBlock *ThenBlock = CGF.createBasicBlock("if.then");
> +  llvm::BasicBlock *ContBlock = CGF.createBasicBlock("if.end");
> +  llvm::BasicBlock *ElseBlock = CGF.createBasicBlock("if.else");
> +
> +  // Emit the condition.
> +  const FunctionDecl *OpDelFD = DE->getOperatorDelete();
> +  llvm::Value *OpDelAddr = CGF.CGM.GetAddrOfFunction(OpDelFD);
> +  //llvm::Function *OpDel = dyn_cast<llvm::Function>(OpDelAddr);
> +  llvm::Value *SDE = CGF.Builder.CreateIsNotNull(OpDelAddr, "sized.del.exists");
> +  CGF.Builder.CreateCondBr(SDE, ThenBlock, ElseBlock);
> +
> +  // Emit the 'then' code.
> +  CGF.EmitBlock(ThenBlock);
> +  EmitDelete(CGF, DE, Ptr, ElementType);
> +  CGF.EmitBranch(ContBlock);
> +
> +  // Compute the 'unsized' delete expr.
> +  CXXDeleteExpr * E = const_cast<CXXDeleteExpr*>(DE);
> +  CXXDeleteExpr *UnsizedDE =
> +  new (CGF.getContext()) CXXDeleteExpr(CGF.getContext().VoidTy,
> +                                       E->isGlobalDelete(),
> +                                       E->isArrayForm(),
> +                                       E->isArrayFormAsWritten(),
> +                                       E->doesUsualArrayDeleteWantSize(),
> +                                       UnsizedDealloc,
> +                                       E->getArgument(),
> +                                       E->getLocStart());

Instead of doing it this way, please introduce a common function to call
from EmitDeleteCall and the CallArrayDelete cleanup.

Doing it this way unnecessarily clones the object / array destruction code,
causes a useless branch when making a virtual call to the deleting destructor,
and fails to branch appropriately within a deleting destructor.

The common function will also be a natural place in the future to check the
language that elides the dynamic check and assumes that the sized
deallocation function exists.  (The reverse option will be implemented by
having Sema just specify an unsized deallocation function.)

John.