Okay, that rule about zero-length arrays applies to destroying

them, too.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135038 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 46 insertions, 60 deletions

diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp
index c30c00455d..62c3a9791d 100644
--- a/lib/CodeGen/CGDecl.cpp
+++ b/lib/CodeGen/CGDecl.cpp
@@ -1174,8 +1174,20 @@ void CodeGenFunction::emitDestroy(llvm::Value *addr, QualType type,
 
   llvm::Value *begin = addr;
   llvm::Value *length = emitArrayLength(arrayType, type, begin);
+
+  // Normally we have to check whether the array is zero-length.
+  bool checkZeroLength = true;
+
+  // But if the array length is constant, we can suppress that.
+  if (llvm::ConstantInt *constLength = dyn_cast<llvm::ConstantInt>(length)) {
+    // ...and if it's constant zero, we can just skip the entire thing.
+    if (constLength->isZero()) return;
+    checkZeroLength = false;
+  }
+
   llvm::Value *end = Builder.CreateInBoundsGEP(begin, length);
-  emitArrayDestroy(begin, end, type, destroyer, useEHCleanupForArray);
+  emitArrayDestroy(begin, end, type, destroyer,
+                   checkZeroLength, useEHCleanupForArray);
 }
 
 /// emitArrayDestroy - Destroys all the elements of the given array,
@@ -1192,6 +1204,7 @@ void CodeGenFunction::emitArrayDestroy(llvm::Value *begin,
                                        llvm::Value *end,
                                        QualType type,
                                        Destroyer &destroyer,
+                                       bool checkZeroLength,
                                        bool useEHCleanup) {
   assert(!type->isArrayType());
 
@@ -1200,6 +1213,12 @@ void CodeGenFunction::emitArrayDestroy(llvm::Value *begin,
   llvm::BasicBlock *bodyBB = createBasicBlock("arraydestroy.body");
   llvm::BasicBlock *doneBB = createBasicBlock("arraydestroy.done");
 
+  if (checkZeroLength) {
+    llvm::Value *isEmpty = Builder.CreateICmpEQ(begin, end,
+                                                "arraydestroy.isempty");
+    Builder.CreateCondBr(isEmpty, doneBB, bodyBB);
+  }
+
   // Enter the loop body, making that address the current address.
   llvm::BasicBlock *entryBB = Builder.GetInsertBlock();
   EmitBlock(bodyBB);
@@ -1232,58 +1251,34 @@ void CodeGenFunction::emitArrayDestroy(llvm::Value *begin,
 
 /// Perform partial array destruction as if in an EH cleanup.  Unlike
 /// emitArrayDestroy, the element type here may still be an array type.
-///
-/// Essentially does an emitArrayDestroy, but checking for the
-/// possibility of a zero-length array (in case the initializer for
-/// the first element throws).
 static void emitPartialArrayDestroy(CodeGenFunction &CGF,
                                     llvm::Value *begin, llvm::Value *end,
                                     QualType type,
                                     CodeGenFunction::Destroyer &destroyer) {
-  // Check whether the array is empty.  For the sake of prettier IR,
-  // we want to jump to the end of the array destroy loop instead of
-  // jumping to yet another block.  We can do this with some modest
-  // assumptions about how emitArrayDestroy works.
-
-  llvm::Value *earlyTest =
-    CGF.Builder.CreateICmpEQ(begin, end, "pad.isempty");
-
-  llvm::BasicBlock *nextBB = CGF.createBasicBlock("pad.arraydestroy");
-
-  // Temporarily, build the conditional branch with identical
-  // successors.  We'll patch this in a bit.
-  llvm::BranchInst *br =
-    CGF.Builder.CreateCondBr(earlyTest, nextBB, nextBB);
-  CGF.EmitBlock(nextBB);
-
-  llvm::Value *zero = llvm::ConstantInt::get(CGF.SizeTy, 0);
-
   // If the element type is itself an array, drill down.
-  llvm::SmallVector<llvm::Value*,4> gepIndices;
-  gepIndices.push_back(zero);
+  unsigned arrayDepth = 0;
   while (const ArrayType *arrayType = CGF.getContext().getAsArrayType(type)) {
     // VLAs don't require a GEP index to walk into.
     if (!isa<VariableArrayType>(arrayType))
-      gepIndices.push_back(zero);
+      arrayDepth++;
     type = arrayType->getElementType();
   }
-  if (gepIndices.size() != 1) {
+
+  if (arrayDepth) {
+    llvm::Value *zero = llvm::ConstantInt::get(CGF.SizeTy, arrayDepth+1);
+
+    llvm::SmallVector<llvm::Value*,4> gepIndices(arrayDepth, zero);
     begin = CGF.Builder.CreateInBoundsGEP(begin, gepIndices.begin(),
                                           gepIndices.end(), "pad.arraybegin");
     end = CGF.Builder.CreateInBoundsGEP(end, gepIndices.begin(),
                                         gepIndices.end(), "pad.arrayend");
   }
 
-  // Now that we know that the array isn't empty, destroy it.  We
-  // don't ever need an EH cleanup because we assume that we're in an
-  // EH cleanup ourselves, so a throwing destructor causes an
-  // immediate terminate.
-  CGF.emitArrayDestroy(begin, end, type, destroyer, /*useEHCleanup*/ false);
-
-  // Set the conditional branch's 'false' successor to doneBB.
-  llvm::BasicBlock *doneBB = CGF.Builder.GetInsertBlock();
-  assert(CGF.Builder.GetInsertPoint() == doneBB->begin());
-  br->setSuccessor(0, doneBB);
+  // Destroy the array.  We don't ever need an EH cleanup because we
+  // assume that we're in an EH cleanup ourselves, so a throwing
+  // destructor causes an immediate terminate.
+  CGF.emitArrayDestroy(begin, end, type, destroyer,
+                       /*checkZeroLength*/ true, /*useEHCleanup*/ false);
 }
 
 namespace {
diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index e3c6a8f978..e8ed84955e 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -1364,26 +1364,16 @@ static void EmitArrayDelete(CodeGenFunction &CGF,
   if (QualType::DestructionKind dtorKind = elementType.isDestructedType()) {
     assert(numElements && "no element count for a type with a destructor!");
 
-    // It's legal to allocate a zero-length array, but emitArrayDestroy
-    // won't handle that correctly, so we need to check that here.
-    llvm::Value *iszero =
-      CGF.Builder.CreateIsNull(numElements, "delete.isempty");
-
-    // We'll patch the 'true' successor of this to lead to the end of
-    // the emitArrayDestroy loop.
-    llvm::BasicBlock *destroyBB = CGF.createBasicBlock("delete.destroy");
-    llvm::BranchInst *br =
-      CGF.Builder.CreateCondBr(iszero, destroyBB, destroyBB);
-    CGF.EmitBlock(destroyBB);
-
     llvm::Value *arrayEnd =
       CGF.Builder.CreateInBoundsGEP(deletedPtr, numElements, "delete.end");
+
+    // Note that it is legal to allocate a zero-length array, and we
+    // can never fold the check away because the length should always
+    // come from a cookie.
     CGF.emitArrayDestroy(deletedPtr, arrayEnd, elementType,
                          CGF.getDestroyer(dtorKind),
+                         /*checkZeroLength*/ true,
                          CGF.needsEHCleanup(dtorKind));
-
-    assert(CGF.Builder.GetInsertBlock()->empty());
-    br->setSuccessor(0, CGF.Builder.GetInsertBlock());
   }
 
   // Pop the cleanup block.
diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index cda1588b1d..322126c811 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -1210,7 +1210,7 @@ public:
                                         bool useEHCleanupForArray);
   void emitArrayDestroy(llvm::Value *begin, llvm::Value *end,
                         QualType type, Destroyer &destroyer,
-                        bool useEHCleanup);
+                        bool checkZeroLength, bool useEHCleanup);
 
   Destroyer &getDestroyer(QualType::DestructionKind destructionKind);
 
diff --git a/test/CodeGenCXX/delete.cpp b/test/CodeGenCXX/delete.cpp
index ae5e29bf06..08ce0de3b5 100644
--- a/test/CodeGenCXX/delete.cpp
+++ b/test/CodeGenCXX/delete.cpp
@@ -75,10 +75,9 @@ namespace test1 {
     // CHECK-NEXT: [[ALLOC:%.*]] = getelementptr inbounds i8* [[T0]], i64 -8
     // CHECK-NEXT: [[T1:%.*]] = bitcast i8* [[ALLOC]] to i64*
     // CHECK-NEXT: [[COUNT:%.*]] = load i64* [[T1]]
-    // CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i64 [[COUNT]], 0
-    // CHECK-NEXT: br i1 [[ISZERO]],
     // CHECK:      [[END:%.*]] = getelementptr inbounds [[A]]* [[BEGIN]], i64 [[COUNT]]
-    // CHECK-NEXT: br label
+    // CHECK-NEXT: [[ISEMPTY:%.*]] = icmp eq [[A]]* [[BEGIN]], [[END]]
+    // CHECK-NEXT: br i1 [[ISEMPTY]],
     // CHECK:      [[PAST:%.*]] = phi [[A]]* [ [[END]], {{%.*}} ], [ [[CUR:%.*]], {{%.*}} ]
     // CHECK-NEXT: [[CUR:%.*]] = getelementptr inbounds [[A]]* [[PAST]], i64 -1
     // CHECK-NEXT: call void @_ZN5test11AD1Ev([[A]]* [[CUR]])
diff --git a/test/CodeGenObjC/arc.m b/test/CodeGenObjC/arc.m
index 55b7747aa6..2431866ef0 100644
--- a/test/CodeGenObjC/arc.m
+++ b/test/CodeGenObjC/arc.m
@@ -511,7 +511,8 @@ void test20(unsigned n) {
 
   // Destroy.
   // CHECK-NEXT: [[END:%.*]] = getelementptr inbounds i8** [[VLA]], i64 [[DIM]]
-  // CHECK-NEXT: br label
+  // CHECK-NEXT: [[EMPTY:%.*]] = icmp eq i8** [[VLA]], [[END]]
+  // CHECK-NEXT: br i1 [[EMPTY]]
 
   // CHECK:      [[AFTER:%.*]] = phi i8** [ [[END]], {{%.*}} ], [ [[CUR:%.*]], {{%.*}} ]
   // CHECK-NEXT: [[CUR:%.*]] = getelementptr inbounds i8** [[AFTER]], i64 -1
@@ -556,7 +557,8 @@ void test21(unsigned n) {
   // CHECK-NEXT: [[BEGIN:%.*]] = getelementptr inbounds [3 x i8*]* [[VLA]], i32 0, i32 0
   // CHECK-NEXT: [[T1:%.*]] = mul nuw i64 [[T0]], 3
   // CHECK-NEXT: [[END:%.*]] = getelementptr inbounds i8** [[BEGIN]], i64 [[T1]]
-  // CHECK-NEXT: br label
+  // CHECK-NEXT: [[EMPTY:%.*]] = icmp eq i8** [[BEGIN]], [[END]]
+  // CHECK-NEXT: br i1 [[EMPTY]]
 
   // CHECK:      [[AFTER:%.*]] = phi i8** [ [[END]], {{%.*}} ], [ [[CUR:%.*]], {{%.*}} ]
   // CHECK-NEXT: [[CUR:%.*]] = getelementptr inbounds i8** [[AFTER]], i64 -1
diff --git a/test/CodeGenObjCXX/arc-new-delete.mm b/test/CodeGenObjCXX/arc-new-delete.mm
index 82747f3b33..4597985f8d 100644
--- a/test/CodeGenObjCXX/arc-new-delete.mm
+++ b/test/CodeGenObjCXX/arc-new-delete.mm
@@ -72,8 +72,8 @@ void test_delete(__strong id *sptr, __weak id *wptr) {
 void test_array_delete(__strong id *sptr, __weak id *wptr) {
   // CHECK: icmp eq i8** [[BEGIN:%.*]], null
   // CHECK: [[LEN:%.*]] = load i64* {{%.*}}
-  // CHECK: icmp eq i64 [[LEN]], 0
   // CHECK: [[END:%.*]] = getelementptr inbounds i8** [[BEGIN]], i64 [[LEN]]
+  // CHECK-NEXT: icmp eq i8** [[BEGIN]], [[END]]
   // CHECK: [[PAST:%.*]] = phi i8** [ [[END]], {{%.*}} ], [ [[CUR:%.*]],
   // CHECK-NEXT: [[CUR]] = getelementptr inbounds i8** [[PAST]], i64 -1
   // CHECK-NEXT: [[T0:%.*]] = load i8** [[CUR]]
@@ -84,8 +84,8 @@ void test_array_delete(__strong id *sptr, __weak id *wptr) {
 
   // CHECK: icmp eq i8** [[BEGIN:%.*]], null
   // CHECK: [[LEN:%.*]] = load i64* {{%.*}}
-  // CHECK: icmp eq i64 [[LEN]], 0
   // CHECK: [[END:%.*]] = getelementptr inbounds i8** [[BEGIN]], i64 [[LEN]]
+  // CHECK-NEXT: icmp eq i8** [[BEGIN]], [[END]]
   // CHECK: [[PAST:%.*]] = phi i8** [ [[END]], {{%.*}} ], [ [[CUR:%.*]],
   // CHECK-NEXT: [[CUR]] = getelementptr inbounds i8** [[PAST]], i64 -1
   // CHECK-NEXT: call void @objc_destroyWeak(i8** [[CUR]])