[cfe-commits] r161100 - in /cfe/trunk: lib/CodeGen/CGClass.cpp test/CodeGenCXX/derived-to-base-conv.cpp test/CodeGenCXX/rvalue-references.cpp

John McCall rjmccall at apple.com
Tue Jul 31 22:04:58 PDT 2012


Author: rjmccall
Date: Wed Aug  1 00:04:58 2012
New Revision: 161100

URL: http://llvm.org/viewvc/llvm-project?rev=161100&view=rev
Log:
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.

Modified:
    cfe/trunk/lib/CodeGen/CGClass.cpp
    cfe/trunk/test/CodeGenCXX/derived-to-base-conv.cpp
    cfe/trunk/test/CodeGenCXX/rvalue-references.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=161100&r1=161099&r2=161100&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Aug  1 00:04:58 2012
@@ -105,30 +105,28 @@
 }
 
 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 @@
   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;
   }
   

Modified: cfe/trunk/test/CodeGenCXX/derived-to-base-conv.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/derived-to-base-conv.cpp?rev=161100&r1=161099&r2=161100&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/derived-to-base-conv.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/derived-to-base-conv.cpp Wed Aug  1 00:04:58 2012
@@ -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
+}

Modified: cfe/trunk/test/CodeGenCXX/rvalue-references.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/rvalue-references.cpp?rev=161100&r1=161099&r2=161100&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/rvalue-references.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/rvalue-references.cpp Wed Aug  1 00:04:58 2012
@@ -10,7 +10,7 @@
 // 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()); }





More information about the cfe-commits mailing list