[PATCH] Abstract out parts of thunk emission code, add support for simple thunks for -cxx-abi microsoft

[Reid Kleckner]

================
Comment at: include/clang/Basic/ABI.h:107
@@ -107,1 +106,3 @@
+  /// \brief Optional ABI-specific data.
+  const void *OpaqueData;
 
----------------
You can avoid the void* by forward declaring CXXMethodDecl.  However, it feels weird to have a soft dependency from Basic to AST.

What are the alternatives to having this here?  It doesn't feel right to keep these out of the comparison operators, even though they are only used to sort the dump.

================
Comment at: lib/CodeGen/CGVTables.cpp:475
@@ -477,1 +474,3 @@
+      !CGM.getCodeGenOpts().OptimizationLevel) {
+    // If the ABI has key functions, we only want to do this when building with optimizations.
     return;
----------------
Can you elaborate on this?  Basically, the thunk should already be available externally if we have key functions, so emitting it to the current TU is just an optimization.

================
Comment at: lib/CodeGen/CGVTables.cpp:499
@@ -497,5 +498,3 @@
   if (VFTContext.isValid()) {
-    // FIXME: This is a temporary solution to force generation of vftables in
-    // Microsoft ABI. Remove when we thread VFTableContext through CodeGen.
-    VFTContext->getVFPtrOffsets(MD->getParent());
-    return;
+    // Don't need to generate thunks for complete destructor in this ABI.
+    if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Complete)
----------------
Can you push this logic into VFTContext->getThunkInfo(MD) and have it return null?  We check for that just below.

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:176
@@ -175,1 +175,3 @@
 
+  bool shouldTryToEmitThunksForExternalVirtualFunctions() { return false; }
+
----------------
Is this dead code?

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:179
@@ +178,3 @@
+  void setThunkLinkage(llvm::Function *Thunk, bool ForVTable) {
+    if (ForVTable)
+      Thunk->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
----------------
Can you elaborate on why it's available externally if this is "for the vtable"?

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:186
@@ -177,1 +185,3 @@
 
+  bool shouldTryToEmitThunksForExternalVirtualFunctions() { return true; }
+
----------------
Dead, I believe


http://llvm-reviews.chandlerc.com/D1787