-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[OpenMP][AIX] Add pthreads and perfstat libraries for linking static libomp.a #167706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-backend-powerpc @llvm/pr-subscribers-clang Author: Kelvin Li (kkwli) ChangesThe libpthreads and perfstat libraries are required on AIX if the libomp.a is a static library. Full diff: https://github.com/llvm/llvm-project/pull/167706.diff 1 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp
index a8acf9cfc44c9..ad32b4f5b16b6 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -309,8 +309,10 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
// Add OpenMP runtime if -fopenmp is specified.
- if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
- options::OPT_fno_openmp, false)) {
+ const bool hasFOpenMP =
+ Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false);
+ if (hasFOpenMP) {
switch (ToolChain.getDriver().getOpenMPRuntime(Args)) {
case Driver::OMPRT_OMP:
CmdArgs.push_back("-lomp");
@@ -325,10 +327,13 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// Already diagnosed.
break;
}
+
+ CmdArgs.push_back("-lperfstat");
}
// Support POSIX threads if "-pthreads" or "-pthread" is present.
- if (Args.hasArg(options::OPT_pthreads, options::OPT_pthread))
+ if (hasFOpenMP ||
+ Args.hasArg(options::OPT_pthreads, options::OPT_pthread))
CmdArgs.push_back("-lpthreads");
if (D.CCCIsCXX())
|
|
@llvm/pr-subscribers-clang-driver Author: Kelvin Li (kkwli) ChangesThe libpthreads and perfstat libraries are required on AIX if the libomp.a is a static library. Full diff: https://github.com/llvm/llvm-project/pull/167706.diff 1 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp
index a8acf9cfc44c9..ad32b4f5b16b6 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -309,8 +309,10 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
// Add OpenMP runtime if -fopenmp is specified.
- if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
- options::OPT_fno_openmp, false)) {
+ const bool hasFOpenMP =
+ Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false);
+ if (hasFOpenMP) {
switch (ToolChain.getDriver().getOpenMPRuntime(Args)) {
case Driver::OMPRT_OMP:
CmdArgs.push_back("-lomp");
@@ -325,10 +327,13 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// Already diagnosed.
break;
}
+
+ CmdArgs.push_back("-lperfstat");
}
// Support POSIX threads if "-pthreads" or "-pthread" is present.
- if (Args.hasArg(options::OPT_pthreads, options::OPT_pthread))
+ if (hasFOpenMP ||
+ Args.hasArg(options::OPT_pthreads, options::OPT_pthread))
CmdArgs.push_back("-lpthreads");
if (D.CCCIsCXX())
|
| break; | ||
| } | ||
|
|
||
| CmdArgs.push_back("-lperfstat"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to tell if this is only added for static libomp.a?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is going to add it for the dynamic links as well, I don't know if that's a behaviour we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielCChen @daltenty
The situation is that libomp.a is a static library (i.e. ar t shows only .o) and we link it by default (i.e. -by option). It fails with missing pthread_* and ptx_* symbols. Adding the two libraries resolves the issues. Before and after the patch, ldd shows:
/usr/lib/libpthreads.a(shr_xpg5_64.o)
/usr/lib/libperfstat.a(shr_64.o)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, there is not doubt that we need this IF the libomp.a is built as a static library. I think the question is if libomp.a is built as a shared library, we probably don't want to link to -lperfstat, and this PR doesn't seem differentiate that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the shared libomp.a is built (i.e. -DLIBOMP_ENABLE_SHARED=ON), it contains only libomp.so.1 as returned by at t. The shared library also has dependency on libperfstat.a.
$ dump -H ./runtimes/runtimes-bins/openmp/runtime/src/libomp.a
./runtimes/runtimes-bins/openmp/runtime/src/libomp.a[libomp.so.1]:
***Loader Section***
...
***Import File Strings***
INDEX PATH BASE MEMBER
0 /home/kli/wrk/f/build-flang/lib/clang/22/lib/powerpc64-ibm-aix:/usr/lib:/lib:
1 libc.a shr_64.o
2 libpthreads.a shr_xpg5_64.o
3 libperfstat.a shr_64.o
For linking this shared library (libomp.a), adding -lperfstat will not change the behavior as far as I know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For dynamic linking, there is no extra dependence based on the output of ldd.
Without -lperfstat,
$ /usr/bin/ld -o a.out -b64 -bpT:0x100000000 -bpD:0x110000000 /usr/lib/crt0_64.o /usr/lib/crti_64.o -bcdtors:all:0:s parallel.o -L/home/kli/llvm-install/bin/../lib/powerpc64-ibm-aix /home/kli/llvm-install/lib/clang/22/lib/powerpc64-ibm-aix/libclang_rt.builtins.a -lunwind -lomp -lc
$ ldd a.out
a.out needs:
/usr/lib/libc.a(shr_64.o)
/home/kli/llvm-install/bin/../lib/powerpc64-ibm-aix/libomp.a(libomp.so.1)
/unix
/usr/lib/libcrypt.a(shr_64.o)
/usr/lib/libpthreads.a(shr_xpg5_64.o)
/usr/lib/libperfstat.a(shr_64.o)
/usr/lib/libcfg.a(shr_64.o)
/usr/lib/libodm.a(shr_64.o)
/usr/lib/liblvm.a(shr_64.o)
/usr/lib/libcorcfg.a(shr_64.o)
/usr/lib/libsrc.a(shr_64.o)
With -lperfstat,
$ /usr/bin/ld -o a.out -b64 -bpT:0x100000000 -bpD:0x110000000 /usr/lib/crt0_64.o /usr/lib/crti_64.o -bcdtors:all:0:s parallel.o -L/home/kli/llvm-install/bin/../lib/powerpc64-ibm-aix /home/kli/llvm-install/lib/clang/22/lib/powerpc64-ibm-aix/libclang_rt.builtins.a -lunwind -lomp -lc -lperfstat
$ ldd a.out
a.out needs:
/usr/lib/libc.a(shr_64.o)
/home/kli/llvm-install/bin/../lib/powerpc64-ibm-aix/libomp.a(libomp.so.1)
/unix
/usr/lib/libcrypt.a(shr_64.o)
/usr/lib/libpthreads.a(shr_xpg5_64.o)
/usr/lib/libperfstat.a(shr_64.o)
/usr/lib/libcfg.a(shr_64.o)
/usr/lib/libodm.a(shr_64.o)
/usr/lib/liblvm.a(shr_64.o)
/usr/lib/libcorcfg.a(shr_64.o)
/usr/lib/libsrc.a(shr_64.o)
| break; | ||
| } | ||
|
|
||
| CmdArgs.push_back("-lperfstat"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is going to add it for the dynamic links as well, I don't know if that's a behaviour we want.
|
If the static |
The libpthreads and perfstat libraries are required on AIX if the libomp.a is a static library.