rnk added inline comments.
================ 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 ---------------- theraven wrote: > compnerd wrote: > > rnk wrote: > > > compnerd wrote: > > > > rnk wrote: > > > > > 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. > > > > I think that @theraven would care more about this code path than I. I > > > > think that this change may be wrong, because the load here is supposed > > > > to be in the ObjC runtime, and this becoming local to the module would > > > > break the global registration. > > > We just set dso_local whenever something isn't dllimport, even if it's a > > > lie sometimes. I think everything would work as intended with a linker > > > thunk. It's "true" as far as LLVM is concerned for the purposes of > > > emitting relocations. > > Ah, okay, then, this might be okay. However, the use of `dllimport` here > > would force that the runtime to be called. Then again it is possible to > > statically link ... > `__objc_load` is a function defined in objc.dll. It absolutely does want to > be dlimport, or the linker won't be able to find it. I don't agree, it doesn't have to be marked dllimport. MSVC, ld.bfd, and lld will all synthesize a linker thunk so that the call works. Marking it dllimport is a performance optimization. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55229/new/ https://reviews.llvm.org/D55229 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
