compnerd added a subscriber: theraven.
compnerd added inline comments.
================
Comment at: CodeGen/CodeGenModule.cpp:2957-2958
!getCodeGenOpts().LTOVisibilityPublicStd &&
- !getTriple().isWindowsGNUEnvironment()) {
+ !getTriple().isWindowsGNUEnvironment() &&
+ !getTriple().isWindowsMSVCEnvironment()) {
const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
----------------
rnk wrote:
> 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.
IIRC, one of the issues was that lldb couldn't deal very well with the thunks.
The other thing is that there is a small penalty for something that we know
that we are going to redirect through. However, I think that if lldb is able
to deal with the thunks now, I would be okay with the penalty to make the
behavior more similar to MSVC. Basically, if lldb works, lets just get rid of
the special behavior for the itanium environment.
================
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
----------------
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.
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
