diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1874,10 +1874,13 @@ } /// Emit the code for deleting a single object. -static void EmitObjectDelete(CodeGenFunction &CGF, +/// \return \c true if we started emitting UnconditionalDeleteBlock, \c false +/// if not. +static bool EmitObjectDelete(CodeGenFunction &CGF, const CXXDeleteExpr *DE, Address Ptr, - QualType ElementType) { + QualType ElementType, + llvm::BasicBlock *UnconditionalDeleteBlock) { // C++11 [expr.delete]p3: // If the static type of the object to be deleted is different from its // dynamic type, the static type shall be a base class of the dynamic type @@ -1924,7 +1927,7 @@ if (UseVirtualCall) { CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType, Dtor); - return; + return false; } } } @@ -1959,7 +1962,15 @@ } } + // When optimizing for size, call 'operator delete' unconditionally. + if (CGF.CGM.getCodeGenOpts().OptimizeSize > 1) { + CGF.EmitBlock(UnconditionalDeleteBlock); + CGF.PopCleanupBlock(); + return true; + } + CGF.PopCleanupBlock(); + return false; } namespace { @@ -2036,6 +2047,12 @@ Address Ptr = EmitPointerWithAlignment(Arg); // Null check the pointer. + // + // We could avoid this null check if we can determine that the object + // destruction is trivial and doesn't require an array cookie; we can + // unconditionally perform the operator delete call in that case. For now, we + // assume that deleted pointers are null rarely enough that it's better to + // keep the branch. This might be worth revisiting for a -O0 code size win. llvm::BasicBlock *DeleteNotNull = createBasicBlock("delete.notnull"); llvm::BasicBlock *DeleteEnd = createBasicBlock("delete.end"); @@ -2081,11 +2098,11 @@ if (E->isArrayForm()) { EmitArrayDelete(*this, E, Ptr, DeleteTy); + EmitBlock(DeleteEnd); } else { - EmitObjectDelete(*this, E, Ptr, DeleteTy); + if (!EmitObjectDelete(*this, E, Ptr, DeleteTy, DeleteEnd)) + EmitBlock(DeleteEnd); } - - EmitBlock(DeleteEnd); } static bool isGLValueFromPointerDeref(const Expr *E) { diff --git a/clang/test/CodeGenCXX/delete.cpp b/clang/test/CodeGenCXX/delete.cpp --- a/clang/test/CodeGenCXX/delete.cpp +++ b/clang/test/CodeGenCXX/delete.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOSIZE +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - -Oz -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,CHECK-SIZE void t1(int *a) { delete a; @@ -9,7 +10,19 @@ }; // POD types. + +// CHECK-LABEL: define void @_Z2t3P1S void t3(S *s) { + // CHECK: icmp {{.*}} null + // CHECK: br i1 + + // CHECK: bitcast + // CHECK-NEXT: call void @_ZdlPv + + // Check the delete is inside the 'if !null' check unless we're optimizing + // for size. FIXME: We could omit the branch entirely in this case. + // CHECK-NOSIZE-NEXT: br + // CHECK-SIZE-NEXT: ret delete s; } @@ -22,7 +35,9 @@ // CHECK-LABEL: define void @_Z2t4P1T void t4(T *t) { // CHECK: call void @_ZN1TD1Ev - // CHECK-NEXT: bitcast + // CHECK-NOSIZE-NEXT: bitcast + // CHECK-SIZE-NEXT: br + // CHECK-SIZE: bitcast // CHECK-NEXT: call void @_ZdlPv delete t; } @@ -49,7 +64,9 @@ // CHECK-LABEL: define void @_ZN5test04testEPNS_1AE( void test(A *a) { // CHECK: call void @_ZN5test01AD1Ev - // CHECK-NEXT: bitcast + // CHECK-NOSIZE-NEXT: bitcast + // CHECK-SIZE-NEXT: br + // CHECK-SIZE: bitcast // CHECK-NEXT: call void @_ZN5test01AdlEPv delete a; } diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -2670,9 +2670,16 @@ // if (foo) free(foo); // into // free(foo); - if (MinimizeSize) - if (Instruction *I = tryToMoveFreeBeforeNullTest(FI, DL)) - return I; + // + // Note that we can only do this for 'free' and not for any flavor of + // 'operator delete'; there is no 'operator delete' symbol for which we are + // permitted to invent a call, even if we're passing in a null pointer. + if (MinimizeSize) { + LibFunc Func; + if (TLI.getLibFunc(FI, Func) && TLI.has(Func) && Func == LibFunc_free) + if (Instruction *I = tryToMoveFreeBeforeNullTest(FI, DL)) + return I; + } return nullptr; } diff --git a/llvm/test/Transforms/InstCombine/malloc-free-delete.ll b/llvm/test/Transforms/InstCombine/malloc-free-delete.ll --- a/llvm/test/Transforms/InstCombine/malloc-free-delete.ll +++ b/llvm/test/Transforms/InstCombine/malloc-free-delete.ll @@ -140,6 +140,31 @@ ret void } +; Same optimization with even a builtin 'operator delete' would be +; incorrect in general. +; 'if (p) delete p;' cannot result in a call to 'operator delete(0)'. +define void @test6a(i8* %foo) minsize { +; CHECK-LABEL: @test6a( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[FOO:%.*]], null +; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]] +; CHECK: if.then: +; CHECK-NEXT: tail call void @_ZdlPv(i8* [[FOO]]) +; CHECK-NEXT: br label [[IF_END]] +; CHECK: if.end: +; CHECK-NEXT: ret void +entry: + %tobool = icmp eq i8* %foo, null + br i1 %tobool, label %if.end, label %if.then + +if.then: ; preds = %entry + tail call void @_ZdlPv(i8* %foo) builtin + br label %if.end + +if.end: ; preds = %entry, %if.then + ret void +} + declare i8* @_ZnwmRKSt9nothrow_t(i64, i8*) nobuiltin declare void @_ZdlPvRKSt9nothrow_t(i8*, i8*) nobuiltin declare i32 @__gxx_personality_v0(...)