Assume loads fix #2

There was linker problem, and it turns out that it is not always safe
to refer to vtable. If the vtable is used, then we can refer to it
without any problem, but because we don't know when it will be used or
not, we can only check if vtable is external or it is safe to to emit it
speculativly (when class it doesn't have any inline virtual functions).
It should be fixed in the future.

http://reviews.llvm.org/D12385

llvm-svn: 246214

clang/lib/CodeGen/CGCXXABI.h

@@ -218,8 +218,10 @@ public:
   virtual void emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) = 0;
   virtual llvm::GlobalVariable *getThrowInfo(QualType T) { return nullptr; }
 
-  virtual bool canEmitAvailableExternallyVTable(
-      const CXXRecordDecl *RD) const = 0;
+  /// \brief Determine whether it's possible to emit a vtable for \p RD, even
+  /// though we do not know that the vtable has been marked as used by semantic
+  /// analysis.
+  virtual bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const = 0;
 
   virtual void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C) = 0;
 

clang/lib/CodeGen/CGClass.cpp

@@ -1857,8 +1857,15 @@ void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
   // with a vtable.  We don't do this for base subobjects for two reasons:
   // first, it's incorrect for classes with virtual bases, and second, we're
   // about to overwrite the vptrs anyway.
+  // We also have to make sure if we can refer to vtable:
+  // - If vtable is external then it's safe to use it (for available_externally
+  //   CGVTables will make sure if it can emit it).
+  // - Otherwise we can refer to vtable if it's safe to speculatively emit.
+  // FIXME: If vtable is used by ctor/dtor, we are always safe to refer to it.
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
-      ClassDecl->isDynamicClass() && Type != Ctor_Base)
+      ClassDecl->isDynamicClass() && Type != Ctor_Base &&
+      (CGM.getVTables().isVTableExternal(ClassDecl) ||
+       CGM.getCXXABI().canSpeculativelyEmitVTable(ClassDecl)))
     EmitVTableAssumptionLoads(ClassDecl, This);
 }
 

clang/lib/CodeGen/CGVTables.cpp

@@ -682,7 +682,7 @@ CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD,
 static bool shouldEmitAvailableExternallyVTable(const CodeGenModule &CGM,
                                                 const CXXRecordDecl *RD) {
   return CGM.getCodeGenOpts().OptimizationLevel > 0 &&
-            CGM.getCXXABI().canEmitAvailableExternallyVTable(RD);
+            CGM.getCXXABI().canSpeculativelyEmitVTable(RD);
 }
 
 /// Compute the required linkage of the v-table for the given class.
@@ -832,11 +832,11 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
 /// we define that v-table?
 static bool shouldEmitVTableAtEndOfTranslationUnit(CodeGenModule &CGM,
                                                    const CXXRecordDecl *RD) {
-  // If vtable is internal then it has to be done
+  // If vtable is internal then it has to be done.
   if (!CGM.getVTables().isVTableExternal(RD))
     return true;
 
-  // If it's external then maybe we will need it as available_externally
+  // If it's external then maybe we will need it as available_externally.
   return shouldEmitAvailableExternallyVTable(CGM, RD);
 }
 

clang/lib/CodeGen/ItaniumCXXABI.cpp

@@ -229,7 +229,7 @@ public:
 
   void emitVirtualInheritanceTables(const CXXRecordDecl *RD) override;
 
-  bool canEmitAvailableExternallyVTable(const CXXRecordDecl *RD) const override;
+  bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const override;
 
   void setThunkLinkage(llvm::Function *Thunk, bool ForVTable, GlobalDecl GD,
                        bool ReturnAdjustment) override {
@@ -1523,8 +1523,7 @@ void ItaniumCXXABI::emitVirtualInheritanceTables(const CXXRecordDecl *RD) {
   VTables.EmitVTTDefinition(VTT, CGM.getVTableLinkage(RD), RD);
 }
 
-bool ItaniumCXXABI::canEmitAvailableExternallyVTable(
-    const CXXRecordDecl *RD) const {
+bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
   // We don't emit available_externally vtables if we are in -fapple-kext mode
   // because kext mode does not permit devirtualization.
   if (CGM.getLangOpts().AppleKext)

clang/lib/CodeGen/MicrosoftCXXABI.cpp

@@ -106,8 +106,7 @@ public:
                                      QualType DestTy) override;
 
   bool EmitBadCastCall(CodeGenFunction &CGF) override;
-  bool canEmitAvailableExternallyVTable(
-      const CXXRecordDecl *RD) const override {
+  bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const override {
     return false;
   }
 

clang/test/CodeGenCXX/template-instantiation.cpp

@@ -1,7 +1,5 @@
 // RUN: %clang_cc1 %s -O1 -disable-llvm-optzns -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s
 
-// CHECK:     @_ZTVN5test018stdio_sync_filebufIA4_iEE = linkonce_odr unnamed_addr constant
-
 // CHECK: @_ZN7PR100011xE = global
 // CHECK-NOT: @_ZN7PR100014kBarE = external global i32
 //
@@ -14,6 +12,7 @@
 // CHECK: @_ZN7PR100011SIiE3arrE = linkonce_odr global [3 x i32]
 // CHECK-NOT: @_ZN7PR100011SIiE3arr2E = linkonce_odr global [3 x i32]A
 
+// CHECK:     @_ZTVN5test018stdio_sync_filebufIA4_iEE = linkonce_odr unnamed_addr constant
 
 // CHECK-NOT: _ZTVN5test31SIiEE
 // CHECK-NOT: _ZTSN5test31SIiEE

clang/test/CodeGenCXX/thunks.cpp

@@ -398,11 +398,13 @@ D::~D() {}
 // Checking with opt
 // CHECK-OPT-LABEL: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(%"struct.Test4B::(anonymous namespace)::C"* %this) unnamed_addr #0 align 2
 
+// This is from Test5:
+// CHECK-OPT-LABEL: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv
+
 // This is from Test10:
 // CHECK-OPT-LABEL: define linkonce_odr void @_ZN6Test101C3fooEv
 // CHECK-OPT-LABEL: define linkonce_odr void @_ZThn8_N6Test101C3fooEv
 
-// This is from Test5:
-// CHECK-OPT-LABEL: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv
+
 
 // CHECK: attributes [[NUW]] = { nounwind uwtable{{.*}} }

clang/test/CodeGenCXX/vtable-assume-load.cpp

@@ -6,7 +6,9 @@
 // RUN: FileCheck --check-prefix=CHECK3 --input-file=%t.ll %s
 // RUN: FileCheck --check-prefix=CHECK4 --input-file=%t.ll %s
 // RUN: FileCheck --check-prefix=CHECK-MS --input-file=%t.ms.ll %s
-
+// RUN: FileCheck --check-prefix=CHECK6 --input-file=%t.ll %s
+// RUN: FileCheck --check-prefix=CHECK7 --input-file=%t.ll %s
+// RUN: FileCheck --check-prefix=CHECK8 --input-file=%t.ll %s
 namespace test1 {
 
 struct A {
@@ -150,7 +152,7 @@ void test() {
 }
 } // test4
 
-namespace test5 {
+namespace testMS {
 
 struct __declspec(novtable) S {
   virtual void foo();
@@ -159,8 +161,8 @@ struct __declspec(novtable) S {
 void g(S &s) { s.foo(); }
 
 // if struct has novtable specifier, then we can't generate assumes
-// CHECK-MS-LABEL: define void @"\01?test@test5@@YAXXZ"()
-// CHECK-MS: call x86_thiscallcc %"struct.test5::S"* @"\01??0S@test5@@QAE@XZ"(
+// CHECK-MS-LABEL: define void @"\01?test@testMS@@YAXXZ"()
+// CHECK-MS: call x86_thiscallcc %"struct.testMS::S"* @"\01??0S@testMS@@QAE@XZ"(
 // CHECK-MS-NOT: @llvm.assume
 // CHECK-MS-LABEL: }
 
@@ -169,4 +171,125 @@ void test() {
   g(s);
 }
 
-} // test5
+} // testMS
+
+namespace test6 {
+// CHECK6: @_ZTVN5test61AE = external
+struct A {
+  A();
+  virtual void foo();
+  virtual ~A() {}
+};
+struct B : A {
+  B();
+};
+// Because A's vtable is external, it's safe to generate assumption loads.
+// CHECK6-LABEL: define void @_ZN5test61gEv()
+// CHECK6: call void @_ZN5test61AC1Ev(
+// CHECK6: call void @llvm.assume(
+
+// We can't emit assumption loads for B, because if we would refer to vtable
+// it would refer to functions that will not be able to find (like implicit
+// inline destructor).
+
+// CHECK6-LABEL:   call void @_ZN5test61BC1Ev(
+// CHECK6-NOT: call void @llvm.assume(
+// CHECK6-LABEL: }
+void g() {
+  A *a = new A;
+  B *b = new B;
+}
+
+}
+
+namespace test7 {
+// Because A's key function is defined here, vtable is generated in this TU
+// CHECK7: @_ZTVN5test71AE = unnamed_addr constant
+struct A {
+  A();
+  virtual void foo();
+  virtual void bar();
+};
+void A::foo() {}
+
+// CHECK7-LABEL: define void @_ZN5test71gEv()
+// CHECK7: call void @_ZN5test71AC1Ev(
+// CHECK7: call void @llvm.assume(
+// CHECK7-LABEL: }
+void g() {
+  A *a = new A();
+  a->bar();
+}
+}
+
+namespace test8 {
+
+struct A {
+  virtual void foo();
+  virtual void bar();
+};
+
+// CHECK8-DAG: @_ZTVN5test81BE = available_externally unnamed_addr constant
+struct B : A {
+  B();
+  void foo();
+  void bar();
+};
+
+// CHECK8-DAG: @_ZTVN5test81CE = linkonce_odr unnamed_addr constant
+struct C : A {
+  C();
+  void bar();
+  void foo() {}
+};
+inline void C::bar() {}
+
+// CHECK8-DAG: @_ZTVN5test81DE = external unnamed_addr constant
+struct D : A {
+  D();
+  void foo();
+  void inline bar();
+};
+void D::bar() {}
+
+// CHECK8-DAG: @_ZTVN5test81EE = linkonce_odr unnamed_addr constant
+struct E : A {
+  E();
+};
+
+// CHECK8-LABEL: define void @_ZN5test81bEv()
+// CHECK8: call void @llvm.assume(
+// CHECK8-LABEL: }
+void b() {
+  B b;
+  b.bar();
+}
+
+// FIXME: C has inline virtual functions which prohibits as from generating
+// assumption loads, but because vtable is generated in this TU (key function
+// defined here) it would be correct to refer to it.
+// CHECK8-LABEL: define void @_ZN5test81cEv()
+// CHECK8-NOT: call void @llvm.assume(
+// CHECK8-LABEL: }
+void c() {
+  C c;
+  c.bar();
+}
+
+// CHECK8-LABEL: define void @_ZN5test81dEv()
+// CHECK8: call void @llvm.assume(
+// CHECK8-LABEL: }
+void d() {
+  D d;
+  d.bar();
+}
+
+// CHECK8-LABEL: define void @_ZN5test81eEv()
+// CHECK8: call void @llvm.assume(
+// CHECK8-LABEL: }
+void e() {
+  E e;
+  e.bar();
+}
+}
+

clang/test/CodeGenCXX/vtable-available-externally.cpp

@@ -184,8 +184,8 @@ void f() {
 }  // Test8
 
 namespace Test9 {
-// all virtual functions are outline, so we can assume that it will
-// be generated in translation unit where foo is defined
+// All virtual functions are outline, so we can assume that it will
+// be generated in translation unit where foo is defined.
 // CHECK-TEST9-DAG: @_ZTVN5Test91AE = available_externally unnamed_addr constant
 // CHECK-TEST9-DAG: @_ZTVN5Test91BE = available_externally unnamed_addr constant
 struct A {
@@ -217,14 +217,14 @@ struct A {
 };
 void A::foo() {}
 
-// Because key function is inline we will generate vtable as linkonce_odr
+// Because key function is inline we will generate vtable as linkonce_odr.
 // CHECK-TEST10-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
 struct D : A {
   void bar();
 };
 inline void D::bar() {}
 
-// because B has outline key function then we can refer to
+// Because B has outline all virtual functions, we can refer to them.
 // CHECK-TEST10-DAG: @_ZTVN6Test101BE = available_externally unnamed_addr constant
 struct B : A {
   void foo();
@@ -233,7 +233,7 @@ struct B : A {
 
 // C's key function (car) is outline, but C has inline virtual function so we
 // can't guarantee that we will be able to refer to bar from name
-// so (at the moment) we can't emit vtable available_externally
+// so (at the moment) we can't emit vtable available_externally.
 // CHECK-TEST10-DAG: @_ZTVN6Test101CE = external unnamed_addr constant
 struct C : A {
   void bar() {}               // defined in body - not key function
@@ -365,4 +365,3 @@ void test() {
 }
 }
 
-