r230580 - Improvement on sized deallocation from r230160:

[John McCall]

> On Feb 26, 2015, at 12:08 PM, John McCall <rjmccall at apple.com> wrote:
> 
>> 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.)

Ping.

John.