llvm/r189852.diff

Don't emit an available_externally vtable pointing to linkonce_odr funcs.

This fixes pr13124.

From the discussion at
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-June/022606.html
we know that we cannot make funcions in a weak_odr vtable also weak_odr. They
should remain linkonce_odr.

The side effect is that we cannot emit a available_externally vtable unless we
also emit a copy of the function. This also has an issue: If codegen is going
to output a function, sema has to mark it used. Given llvm.org/pr9114, it looks
like sema cannot be more aggressive at marking functions used because
of vtables.

This leaves us with a few unpleasant options:

* Marking functions in vtables used if possible. This sounds a bit sloppy, so
  we should avoid it.
* Producing available_externally vtables only when all the functions in it are
  already used or weak_odr. This would cover cases like

--------------------
struct foo {
  virtual ~foo();
};
struct bar : public foo {
  virtual void zed();
};
void f() {
  foo *x(new bar);
  delete x;
}
void g(bar *x) {
  x->~bar(); // force the destructor to be used
}
--------------------------

and

----------------------------------
template<typename T>
struct bar {
  virtual ~bar();
};
template<typename T>
bar<T>::~bar() {
}

// make the destructor weak_odr instead of linkonce_odr
extern template class bar<int>;

void f() {
  bar<int> *x(new bar<int>);
  delete x;
}
----------------------------

These look like corner cases, so it is unclear if it is worth it.

* And finally: Just nuke this optimization. That is what this patch implements.


--- tools/clang/lib/CodeGen/CGVTables.cpp
+++ tools/clang/lib/CodeGen/CGVTables.cpp
@@ -734,12 +734,7 @@
     switch (keyFunction->getTemplateSpecializationKind()) {
       case TSK_Undeclared:
       case TSK_ExplicitSpecialization:
-        // When compiling with optimizations turned on, we emit all vtables,
-        // even if the key function is not defined in the current translation
-        // unit. If this is the case, use available_externally linkage.
-        if (!def && CodeGenOpts.OptimizationLevel)
-          return llvm::GlobalVariable::AvailableExternallyLinkage;
-
+        assert(def && "Should not have been asked to emit this");
         if (keyFunction->isInlined())
           return !Context.getLangOpts().AppleKext ?
                    llvm::GlobalVariable::LinkOnceODRLinkage :
@@ -758,9 +753,7 @@
                  llvm::Function::InternalLinkage;
   
       case TSK_ExplicitInstantiationDeclaration:
-        return !Context.getLangOpts().AppleKext ?
-                 llvm::GlobalVariable::AvailableExternallyLinkage :
-                 llvm::Function::InternalLinkage;
+        llvm_unreachable("Should not have been asked to emit this");
     }
   }
 
@@ -776,7 +769,7 @@
     return llvm::GlobalVariable::LinkOnceODRLinkage;
 
   case TSK_ExplicitInstantiationDeclaration:
-    return llvm::GlobalVariable::AvailableExternallyLinkage;
+    llvm_unreachable("Should not have been asked to emit this");
 
   case TSK_ExplicitInstantiationDefinition:
       return llvm::GlobalVariable::WeakODRLinkage;
@@ -875,16 +868,6 @@
 /// we define that v-table?
 static bool shouldEmitVTableAtEndOfTranslationUnit(CodeGenModule &CGM,
                                                    const CXXRecordDecl *RD) {
-  // If we're building with optimization, we always emit v-tables
-  // since that allows for virtual function calls to be devirtualized.
-  // If the v-table is defined strongly elsewhere, this definition
-  // will be emitted available_externally.
-  //
-  // However, we don't want to do this in -fapple-kext mode, because
-  // kext mode does not permit devirtualization.
-  if (CGM.getCodeGenOpts().OptimizationLevel && !CGM.getLangOpts().AppleKext)
-    return true;
-
   return !CGM.getVTables().isVTableExternal(RD);
 }
 
--- tools/clang/test/CodeGenCXX/thunks-available-externally.cpp
+++ tools/clang/test/CodeGenCXX/thunks-available-externally.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -O3 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm-only -O3
 
 // Check that we don't assert on this case.
 namespace Test1 {
@@ -58,15 +58,6 @@
   b->f();
 }
 
-// CHECK: define void @_ZN5Test21fEv()
-// CHECK: call void @_ZN5Test21C1fEv
-// CHECK: ret void
-// CHECK: define available_externally void @_ZThn16_N5Test21C1fEv
-void f() {
-  C c;
-  f(&c);
-}
-
 }
 
 // Test that we don't assert.
--- tools/clang/test/CodeGenCXX/vtable-available-externally.cpp
+++ tools/clang/test/CodeGenCXX/vtable-available-externally.cpp
@@ -2,18 +2,10 @@
 // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST2 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST5 %s < %t
-// RUN: FileCheck --check-prefix=CHECK-TEST7 %s < %t
 
 #include <typeinfo>
 
-// Test1::A's key function (f) is not defined in this translation
-// unit, but in order to devirtualize calls, we emit the v-table with
-// available_externally linkage.
-//
-// There's no real reason to do this to the RTTI, though.
-
-// CHECK-TEST1: @_ZTVN5Test11AE = available_externally
-// CHECK-TEST1: @_ZTIN5Test11AE = external constant i8*
+// CHECK-TEST1: @_ZTVN5Test11AE = external unnamed_addr constant
 namespace Test1 {
 
 struct A {
@@ -160,13 +152,4 @@
   void f6 ();
 };
 
-// CHECK-TEST7: define void @_ZN5Test79check_c28Ev
-// CHECK-TEST7: call void @_ZN5Test73c282f6Ev
-// CHECK-TEST7: ret void
-void check_c28 () {
-  c28 obj;
-  c11 *ptr = &obj;
-  ptr->f6 ();
-}
-
 }
--- tools/clang/test/CodeGenCXX/vtable-linkage.cpp
+++ tools/clang/test/CodeGenCXX/vtable-linkage.cpp
@@ -163,7 +163,7 @@
 // F<int> is an explicit template instantiation declaration without a
 // key function, so its vtable should have external linkage.
 // CHECK-9: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-9-OPT: @_ZTV1FIiE = available_externally unnamed_addr constant
+// CHECK-9-OPT: @_ZTV1FIiE = external unnamed_addr constant
 
 // E<int> is an explicit template instantiation declaration. It has a
 // key function that is not instantiated, so we should only reference