qiongsiwu1 marked 2 inline comments as done.
qiongsiwu1 added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440
+ if (NeedsProfileRT || needsGCovInstrumentation(Args))
+ CmdArgs.push_back("-lpthreads");
+
----------------
w2yehia wrote:
> w2yehia wrote:
> > The change in `compiler-rt/lib/profile/InstrProfilingFile.c` affects
> > non-AIX platforms too, so won't they need `-lpthreads` too?
> >
> In order to avoid adding -lpthreads, we can leave it to the user to decide
> whether to add or not (i.e. they would add -lpthreads to their application's
> link command if they want to).
> And so to avoid a link error due to undefined symbol, we can define
> `pthread_atfork` as a weak empty stub in the compiler-rt.
> The change in `compiler-rt/lib/profile/InstrProfilingFile.c` affects non-AIX
> platforms too, so won't they need `-lpthreads` too?
>
This is a good question. On Linux we are ok because `pthread_atfork` is
actually implemented by `__register_atfork`, which is available in `libc.so`,
so we do not need `-lpthreads`. I am not sure about other platforms. @davidxl
@vsk @ellis may I get your comments?
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440
+ if (NeedsProfileRT || needsGCovInstrumentation(Args))
+ CmdArgs.push_back("-lpthreads");
+
----------------
qiongsiwu1 wrote:
> w2yehia wrote:
> > w2yehia wrote:
> > > The change in `compiler-rt/lib/profile/InstrProfilingFile.c` affects
> > > non-AIX platforms too, so won't they need `-lpthreads` too?
> > >
> > In order to avoid adding -lpthreads, we can leave it to the user to decide
> > whether to add or not (i.e. they would add -lpthreads to their
> > application's link command if they want to).
> > And so to avoid a link error due to undefined symbol, we can define
> > `pthread_atfork` as a weak empty stub in the compiler-rt.
> > The change in `compiler-rt/lib/profile/InstrProfilingFile.c` affects
> > non-AIX platforms too, so won't they need `-lpthreads` too?
> >
>
> This is a good question. On Linux we are ok because `pthread_atfork` is
> actually implemented by `__register_atfork`, which is available in `libc.so`,
> so we do not need `-lpthreads`. I am not sure about other platforms. @davidxl
> @vsk @ellis may I get your comments?
> In order to avoid adding -lpthreads, we can leave it to the user to decide
> whether to add or not (i.e. they would add -lpthreads to their application's
> link command if they want to).
> And so to avoid a link error due to undefined symbol, we can define
> `pthread_atfork` as a weak empty stub in the compiler-rt.
@w2yehia and I discussed offline, and we think it is ok to pass in `-lpthread`
on AIX. There should not be any functional issues.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155290/new/
https://reviews.llvm.org/D155290
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
