When devirtualizing the conversion to a virtual base subobject,

don't explode if the offset we get is zero.  This can happen if
you have an empty virtual base class.

While I'm at it, remove an unnecessary block from the IR-generation
of the null-check, mark the eventual GEP as inbounds, and generally
prettify.

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

3 files changed, 121 insertions, 113 deletions

diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp
index b53f139293..332033cfc4 100644
--- a/lib/CodeGen/CGClass.cpp
+++ b/lib/CodeGen/CGClass.cpp
@@ -105,30 +105,28 @@ CodeGenFunction::GetAddressOfDirectBaseInCompleteClass(llvm::Value *This,
 }
 
 static llvm::Value *
-ApplyNonVirtualAndVirtualOffset(CodeGenFunction &CGF, llvm::Value *ThisPtr,
-                                CharUnits NonVirtual, llvm::Value *Virtual) {
-  llvm::Type *PtrDiffTy = 
-    CGF.ConvertType(CGF.getContext().getPointerDiffType());
-  
-  llvm::Value *NonVirtualOffset = 0;
-  if (!NonVirtual.isZero())
-    NonVirtualOffset = llvm::ConstantInt::get(PtrDiffTy, 
-                                              NonVirtual.getQuantity());
-  
-  llvm::Value *BaseOffset;
-  if (Virtual) {
-    if (NonVirtualOffset)
-      BaseOffset = CGF.Builder.CreateAdd(Virtual, NonVirtualOffset);
-    else
-      BaseOffset = Virtual;
-  } else
-    BaseOffset = NonVirtualOffset;
+ApplyNonVirtualAndVirtualOffset(CodeGenFunction &CGF, llvm::Value *ptr,
+                                CharUnits nonVirtualOffset,
+                                llvm::Value *virtualOffset) {
+  // Assert that we have something to do.
+  assert(!nonVirtualOffset.isZero() || virtualOffset != 0);
+
+  // Compute the offset from the static and dynamic components.
+  llvm::Value *baseOffset;
+  if (!nonVirtualOffset.isZero()) {
+    baseOffset = llvm::ConstantInt::get(CGF.PtrDiffTy,
+                                        nonVirtualOffset.getQuantity());
+    if (virtualOffset) {
+      baseOffset = CGF.Builder.CreateAdd(virtualOffset, baseOffset);
+    }
+  } else {
+    baseOffset = virtualOffset;
+  }
   
   // Apply the base offset.
-  ThisPtr = CGF.Builder.CreateBitCast(ThisPtr, CGF.Int8PtrTy);
-  ThisPtr = CGF.Builder.CreateGEP(ThisPtr, BaseOffset, "add.ptr");
-
-  return ThisPtr;
+  ptr = CGF.Builder.CreateBitCast(ptr, CGF.Int8PtrTy);
+  ptr = CGF.Builder.CreateInBoundsGEP(ptr, baseOffset, "add.ptr");
+  return ptr;
 }
 
 llvm::Value *
@@ -142,72 +140,81 @@ CodeGenFunction::GetAddressOfBaseClass(llvm::Value *Value,
   CastExpr::path_const_iterator Start = PathBegin;
   const CXXRecordDecl *VBase = 0;
   
-  // Get the virtual base.
+  // Sema has done some convenient canonicalization here: if the
+  // access path involved any virtual steps, the conversion path will
+  // *start* with a step down to the correct virtual base subobject,
+  // and hence will not require any further steps.
   if ((*Start)->isVirtual()) {
     VBase = 
       cast<CXXRecordDecl>((*Start)->getType()->getAs<RecordType>()->getDecl());
     ++Start;
   }
-  
+
+  // Compute the static offset of the ultimate destination within its
+  // allocating subobject (the virtual base, if there is one, or else
+  // the "complete" object that we see).
   CharUnits NonVirtualOffset = 
     ComputeNonVirtualBaseClassOffset(getContext(), VBase ? VBase : Derived,
                                      Start, PathEnd);
 
+  // If there's a virtual step, we can sometimes "devirtualize" it.
+  // For now, that's limited to when the derived type is final.
+  // TODO: "devirtualize" this for accesses to known-complete objects.
+  if (VBase && Derived->hasAttr<FinalAttr>()) {
+    const ASTRecordLayout &layout = getContext().getASTRecordLayout(Derived);
+    CharUnits vBaseOffset = layout.getVBaseClassOffset(VBase);
+    NonVirtualOffset += vBaseOffset;
+    VBase = 0; // we no longer have a virtual step
+  }
+
   // Get the base pointer type.
   llvm::Type *BasePtrTy = 
     ConvertType((PathEnd[-1])->getType())->getPointerTo();
-  
+
+  // If the static offset is zero and we don't have a virtual step,
+  // just do a bitcast; null checks are unnecessary.
   if (NonVirtualOffset.isZero() && !VBase) {
-    // Just cast back.
     return Builder.CreateBitCast(Value, BasePtrTy);
   }    
+
+  llvm::BasicBlock *origBB = 0;
+  llvm::BasicBlock *endBB = 0;
   
-  llvm::BasicBlock *CastNull = 0;
-  llvm::BasicBlock *CastNotNull = 0;
-  llvm::BasicBlock *CastEnd = 0;
-  
+  // Skip over the offset (and the vtable load) if we're supposed to
+  // null-check the pointer.
   if (NullCheckValue) {
-    CastNull = createBasicBlock("cast.null");
-    CastNotNull = createBasicBlock("cast.notnull");
-    CastEnd = createBasicBlock("cast.end");
+    origBB = Builder.GetInsertBlock();
+    llvm::BasicBlock *notNullBB = createBasicBlock("cast.notnull");
+    endBB = createBasicBlock("cast.end");
     
-    llvm::Value *IsNull = Builder.CreateIsNull(Value);
-    Builder.CreateCondBr(IsNull, CastNull, CastNotNull);
-    EmitBlock(CastNotNull);
+    llvm::Value *isNull = Builder.CreateIsNull(Value);
+    Builder.CreateCondBr(isNull, endBB, notNullBB);
+    EmitBlock(notNullBB);
   }
 
+  // Compute the virtual offset.
   llvm::Value *VirtualOffset = 0;
-
   if (VBase) {
-    if (Derived->hasAttr<FinalAttr>()) {
-      VirtualOffset = 0;
-
-      const ASTRecordLayout &Layout = getContext().getASTRecordLayout(Derived);
-
-      CharUnits VBaseOffset = Layout.getVBaseClassOffset(VBase);
-      NonVirtualOffset += VBaseOffset;
-    } else
-      VirtualOffset = GetVirtualBaseClassOffset(Value, Derived, VBase);
+    VirtualOffset = GetVirtualBaseClassOffset(Value, Derived, VBase);
   }
 
-  // Apply the offsets.
+  // Apply both offsets.
   Value = ApplyNonVirtualAndVirtualOffset(*this, Value, 
                                           NonVirtualOffset,
                                           VirtualOffset);
   
-  // Cast back.
+  // Cast to the destination type.
   Value = Builder.CreateBitCast(Value, BasePtrTy);
- 
+
+  // Build a phi if we needed a null check.
   if (NullCheckValue) {
-    Builder.CreateBr(CastEnd);
-    EmitBlock(CastNull);
-    Builder.CreateBr(CastEnd);
-    EmitBlock(CastEnd);
+    llvm::BasicBlock *notNullBB = Builder.GetInsertBlock();
+    Builder.CreateBr(endBB);
+    EmitBlock(endBB);
     
-    llvm::PHINode *PHI = Builder.CreatePHI(Value->getType(), 2);
-    PHI->addIncoming(Value, CastNotNull);
-    PHI->addIncoming(llvm::Constant::getNullValue(Value->getType()), 
-                     CastNull);
+    llvm::PHINode *PHI = Builder.CreatePHI(BasePtrTy, 2, "cast.result");
+    PHI->addIncoming(Value, notNullBB);
+    PHI->addIncoming(llvm::Constant::getNullValue(BasePtrTy), origBB);
     Value = PHI;
   }
   
diff --git a/test/CodeGenCXX/derived-to-base-conv.cpp b/test/CodeGenCXX/derived-to-base-conv.cpp
index 8c51809e04..9b15c6804d 100644
--- a/test/CodeGenCXX/derived-to-base-conv.cpp
+++ b/test/CodeGenCXX/derived-to-base-conv.cpp
@@ -1,85 +1,86 @@
-// REQUIRES: x86-registered-target,x86-64-registered-target
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -S %s -o %t-64.s
-// RUN: FileCheck -check-prefix LP64 --input-file=%t-64.s %s
-// RUN: %clang_cc1 -triple i386-apple-darwin -std=c++11 -S %s -o %t-32.s
-// RUN: FileCheck -check-prefix LP32 --input-file=%t-32.s %s
-
-extern "C" int printf(...);
-extern "C" void exit(int);
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm %s -o - | FileCheck %s
 
 struct A {
-  A (const A&) { printf("A::A(const A&)\n"); }
-  A() {};
-  ~A() { printf("A::~A()\n"); }
+  A(const A&);
+  A();
+  ~A();
 }; 
 
 struct B : public A {
-  B() {};
-  B(const B& Other) : A(Other) { printf("B::B(const B&)\n"); }
-  ~B() { printf("B::~B()\n"); }
+  B();
+  B(const B& Other);
+  ~B();
 };
 
 struct C : public B {
-  C() {};
-  C(const C& Other) : B(Other) { printf("C::C(const C&)\n"); }
-  ~C() { printf("C::~C()\n"); }
+  C();
+  C(const C& Other);
+  ~C();
 }; 
 
 struct X {
-	operator B&() {printf("X::operator B&()\n"); return b; }
-	operator C&() {printf("X::operator C&()\n"); return c; }
- 	X (const X&) { printf("X::X(const X&)\n"); }
- 	X () { printf("X::X()\n"); }
- 	~X () { printf("X::~X()\n"); }
-	B b;
-	C c;
+  operator B&();
+  operator C&();
+  X(const X&);
+  X();
+  ~X();
+  B b;
+  C c;
 };
 
-void f(A) {
-  printf("f(A)\n");
-}
-
-
-void func(X x) 
-{
-  f (x);
-}
-
-int main()
-{
-    X x;
-    func(x);
+void test0_helper(A);
+void test0(X x) {
+  test0_helper(x);
+  // CHECK:    define void @_Z5test01X(
+  // CHECK:      [[TMP:%.*]] = alloca [[A:%.*]], align
+  // CHECK-NEXT: [[T0:%.*]] = call [[B:%.*]]* @_ZN1XcvR1BEv(
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[B]]* [[T0]] to [[A]]*
+  // CHECK-NEXT: call void @_ZN1AC1ERKS_([[A]]* [[TMP]], [[A]]* [[T1]])
+  // CHECK-NEXT: call void @_Z12test0_helper1A([[A]]* [[TMP]])
+  // CHECK-NEXT: call void @_ZN1AD1Ev([[A]]* [[TMP]])
+  // CHECK-NEXT: ret void
 }
 
 struct Base;
 
 struct Root {
-  operator Base&() { exit(1); }
+  operator Base&();
 };
 
 struct Derived;
 
 struct Base : Root {
-  Base(const Base&) { printf("Base::(const Base&)\n"); }
-  Base() { printf("Base::Base()\n"); }
-  operator Derived&() { exit(1); }
+  Base(const Base &);
+  Base();
+  operator Derived &();
 };
 
 struct Derived : Base {
 };
 
-void foo(Base) {}
-
-void test(Derived bb)
-{
-	// CHECK-LP64-NOT: callq    __ZN4BasecvR7DerivedEv
-	// CHECK-LP32-NOT: callq    L__ZN4BasecvR7DerivedEv
-        foo(bb);
+void test1_helper(Base);
+void test1(Derived bb) {
+  // CHECK:     define void @_Z5test17Derived(
+  // CHECK-NOT: call {{.*}} @_ZN4BasecvR7DerivedEv(
+  // CHECK:     call void @_ZN4BaseC1ERKS_(
+  // CHECK-NOT: call {{.*}} @_ZN4BasecvR7DerivedEv(
+  // CHECK:     call void @_Z12test1_helper4Base(
+  test1_helper(bb);
 }
-// CHECK-LP64: callq    __ZN1XcvR1BEv
-// CHECK-LP64: callq    __ZN1AC1ERKS_
-
-// CHECK-LP32: calll     L__ZN1XcvR1BEv
-// CHECK-LP32: calll     L__ZN1AC1ERKS_
-
 
+// Don't crash after devirtualizing a derived-to-base conversion
+// to an empty base allocated at offset zero.
+// rdar://problem/11993704
+class Test2a {};
+class Test2b final : public virtual Test2a {};
+void test2(Test2b &x) {
+  Test2a &y = x;
+  // CHECK:    define void @_Z5test2R6Test2b(
+  // CHECK:      [[X:%.*]] = alloca [[B:%.*]]*, align 8
+  // CHECK-NEXT: [[Y:%.*]] = alloca [[A:%.*]]*, align 8
+  // CHECK-NEXT: store [[B]]* {{%.*}}, [[B]]** [[X]], align 8
+  // CHECK-NEXT: [[T0:%.*]] = load [[B]]** [[X]], align 8
+  // CHECK-NEXT: [[T1:%.*]] = bitcast [[B]]* [[T0]] to [[A]]*
+  // CHECK-NEXT: store [[A]]* [[T1]], [[A]]** [[Y]], align 8
+  // CHECK-NEXT: ret void
+}
diff --git a/test/CodeGenCXX/rvalue-references.cpp b/test/CodeGenCXX/rvalue-references.cpp
index 1c25543bea..b8d47dcfbe 100644
--- a/test/CodeGenCXX/rvalue-references.cpp
+++ b/test/CodeGenCXX/rvalue-references.cpp
@@ -10,7 +10,7 @@ B &getB();
 // CHECK: define %struct.A* @_Z4getAv()
 // CHECK: call %struct.B* @_Z4getBv()
 // CHECK-NEXT: bitcast %struct.B*
-// CHECK-NEXT: getelementptr i8*
+// CHECK-NEXT: getelementptr inbounds i8*
 // CHECK-NEXT: bitcast i8* {{.*}} to %struct.A*
 // CHECK-NEXT: ret %struct.A*
 A &&getA() { return static_cast<A&&>(getB()); }