Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions clang/lib/Driver/ToolChains/AIX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}

// Add directory to library search path.
bool hasFOpenMP = false;
Args.AddAllArgs(CmdArgs, options::OPT_L);
if (!Args.hasArg(options::OPT_r)) {
ToolChain.AddFilePathLibArgs(Args, CmdArgs);
Expand All @@ -309,8 +310,9 @@ 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)) {
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");
Expand All @@ -325,10 +327,13 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// Already diagnosed.
break;
}

CmdArgs.push_back("-lperfstat");
Copy link
Contributor

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?

Copy link
Member

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.

Copy link
Collaborator Author

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)

Copy link
Contributor

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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)

}

// 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())
Expand All @@ -350,7 +355,9 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
ToolChain.addFortranRuntimeLibraryPath(Args, CmdArgs);
ToolChain.addFortranRuntimeLibs(Args, CmdArgs);
CmdArgs.push_back("-lm");
CmdArgs.push_back("-lpthread");
// If the -fopenmp option is specified, no need to add it again.
if (!hasFOpenMP)
CmdArgs.push_back("-lpthreads");
}
const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
Expand Down
Loading