[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Sorry, I was out sick for a week. We should ask @smeenai about this change, since he has been doing more work in this area recently.



================
Comment at: CodeGen/CodeGenModule.cpp:2957-2958
           !getCodeGenOpts().LTOVisibilityPublicStd &&
-          !getTriple().isWindowsGNUEnvironment()) {
+          !getTriple().isWindowsGNUEnvironment() &&
+          !getTriple().isWindowsMSVCEnvironment()) {
         const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
----------------
The condition here of `T.isOSBinFormatCOFF() && !T.isWindowsGNUEnvironment() && !T.isWindowsMSVCEnvironment()` is essentially equivalent to `T.isWindowsItaniumEnvironment()`. Please simplify the condition to that. The other relevant environments are Cygnus for Cygwin and CoreCLR, which probably want to follow the MSVC/GNU behavior.


================
Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 
----------------
mgrang wrote:
> I had to remove dllimport in this and the next unit test. I am not sure of the wider implications of this change. Maybe controlling this via a flag (like -flto-visibility-public-std) is a better/more localized option?
These are the ones that I think @compnerd really cares about since the objc runtime is typically dynamically linked and frequently called. We might want to arrange things such that we have a separate codepath for ObjC runtime helpers. I'm surprised we don't already have such a code path.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55229/new/

https://reviews.llvm.org/D55229