[PATCH] [-cxx-abi microsoft] Emit thunks for pointers to virtual member functions

[Reid Kleckner]

  LGTM


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1434
@@ +1433,3 @@
+      FirstField = llvm::Constant::getNullValue(CGM.VoidPtrTy);
+    } else if (CGM.getMicrosoftVTableContext()
+                   .getMethodVFTableLocation(MD)
----------------
Timur Iskhodzhanov wrote:
> how about
> 
>   } else {
>     ... &ML = CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
>     if (ML.VBase) {
>        ...
>     } else {
>        ...
>     }
>   }
> 
> ?
+1

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:988-991
@@ +987,6 @@
+  llvm::Function *ThunkFn =
+      llvm::Function::Create(ThunkTy, llvm::Function::ExternalLinkage,
+                             ThunkName.str(), &CGM.getModule());
+  assert(ThunkFn->getName() == ThunkName && "name was uniqued!");
+  ThunkFn->setLinkage(llvm::GlobalValue::LinkOnceODRLinkage);
+  CGM.SetLLVMFunctionAttributes(MD, FnInfo, ThunkFn);
----------------
Can you create the function with linkonce_odr instead of setting it later?

Also, I think if the class type has internal linkage we need to make the thunk internal, or the thunk name could collide with a thunk from a different TU.  I don't think the language allows you to compare them, but internal is always stronger than linkonce_odr for optimizers, so it's good to apply it when possible.

IMO the right thing is:

  llvm::GlobalValue::LinkageType L = MD->isExternallyVisible()
                                         ? llvm::GlobalValue::LinkOnceODRLinkage
                                         : llvm::GlobalValue::InternalLinkage;



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