clang 10.0.0_rc1 standalone fails to build with polly

[zmodem]

Bugzilla Link	44870
Resolution	FIXED
Resolved on	Jun 17, 2020 17:49
Version	10.0
OS	All
Blocks	#44654
CC	@andrewrk,@jpalus,@Meinersbur,@FabioPedretti,@serge-sans-paille,@sylvestre,@timotheecour,@tjaalton,@tobiasgrosser,@tstellar
Fixed by commit(s)	d21664c 8f766e3 f149195 5f510e5

Extended Description

Originally filed as #120

[zmodem]

Serge has a patch out here: https://reviews.llvm.org/D74464

[zmodem]

Based on #120 it sounds like there are still issues.

[zmodem]

I'm not going to block on this.

[sylvestre]

Hans, FYI, we have to take the patch in Debian/Ubuntu packages to fix a link issue with mesa
should be probably in 10.0.1

[zmodem]

Hans, FYI, we have to take the patch in Debian/Ubuntu packages to fix a link
issue with mesa
should be probably in 10.0.1

From #191 it sounds like it's not clear that D74464 helps?

[sylvestre]

Oh, having two bug trackers isn't optimal :(

Anyway, the patch I am talking about hasn't been uploaded yet

[tjaalton]

I've tested the package with the patch, and it didn't help. So I'm still unable to build mesa, and now hitting the same bug trying to build intel-opencl-clang.

[sylvestre]

Could you please provide a test case which would not be "rebuild mesa" ? :)
thanks

[tjaalton]

Well, build intel-opencl-clang instead? Much quicker..

https://salsa.debian.org/opencl-team/intel-opencl-clang

[Meinersbur]

Could you check whether 3a0f6e6 and 87dac7d fix the issue?

[tjaalton]

thanks, I will!

[tjaalton]

nope, didn't help, still get the same error..

[sylvestre]

Jan Palus shared this STR with me:

$ cat test.cpp
#include <clang/CodeGen/BackendUtil.h>

using namespace clang;

int main() {
DiagnosticsEngine* diags;
HeaderSearchOptions* hsOpts;
CodeGenOptions* cgOpts;
TargetOptions* tOpts;
LangOptions* lOpts;
llvm::DataLayout* tDesc;
llvm::Module* m;
BackendAction* action;
std::unique_ptr<raw_pwrite_stream> AsmOutStream;

EmitBackendOutput(*diags, *hsOpts, *cgOpts, *tOpts, *lOpts, *tDesc, m, *action, std::move(AsmOutStream));
}

$ g++ test.cpp -o test -lclangBasic -lclangCodeGen -lclangDriver -lclangFrontend -lclangFrontendTool -lclangCodeGen -lclangRewriteFrontend -lclangARCMigrate -lclangStaticAnalyzerFrontend -lclangStaticAnalyzerCheckers -lclangStaticAnalyzerCore -lclangCrossTU -lclangIndex -lclangFrontend -lclangDriver -lclangParse -lclangSerialization -lclangSema -lclangAnalysis -lclangEdit -lclangFormat -lclangToolingInclusions -lclangToolingCore -lclangRewrite -lclangASTMatchers -lclangAST -lclangLex -lclangBasic -ldl -lLLVMSPIRVLib /usr/lib64/libLLVM-10.so -lclangCodeGen -lclangDriver -lclangFrontend -lclangFrontendTool -lclangRewriteFrontend -lclangARCMigrate -lclangStaticAnalyzerFrontend -lclangStaticAnalyzerCheckers -lclangStaticAnalyzerCore -lclangCrossTU -lclangIndex -lclangParse -lclangSerialization -lclangSema -lclangAnalysis -lclangEdit -lclangFormat -lclangToolingInclusions -lclangToolingCore -lclangRewrite -lclangASTMatchers -lclangAST -lclangLex -ldl -lLLVMSPIRVLib
/usr/bin/ld: /usr/lib64/gcc/x86_64-pld-linux/9.3.0/../../../../lib64/libclangCodeGen.a(BackendUtil.cpp.o): in function (anonymous namespace)::EmitAssemblyHelper::EmitAssemblyWithNewPassManager(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >)': BackendUtil.cpp:(.text._ZN12_GLOBAL__N_118EmitAssemblyHelper30EmitAssemblyWithNewPassManagerEN5clang13BackendActionESt10unique_ptrIN4llvm17raw_pwrite_streamESt14default_deleteIS5_EE+0x128b): undefined reference to getPollyPluginInfo()'
collect2: error: ld returned 1 exit status

[sylvestre]

With Debian packages, the command is:
$ g++ a.cpp -o test -lclangBasic -lclangCodeGen -lclangDriver -lclangFrontend -lclangFrontendTool -lclangCodeGen -lclangRewriteFrontend -lclangARCMigrate -lclangStaticAnalyzerFrontend -lclangStaticAnalyzerCheckers -lclangStaticAnalyzerCore -lclangCrossTU -lclangIndex -lclangFrontend -lclangDriver -lclangParse -lclangSerialization -lclangSema -lclangAnalysis -lclangEdit -lclangFormat -lclangToolingInclusions -lclangToolingCore -lclangRewrite -lclangASTMatchers -lclangAST -lclangLex -lclangBasic -ldl /usr/lib/llvm-10/lib/libLLVM-10.so -lclangCodeGen -lclangDriver -lclangFrontend -lclangFrontendTool -lclangRewriteFrontend -lclangARCMigrate -lclangStaticAnalyzerFrontend -lclangStaticAnalyzerCheckers -lclangStaticAnalyzerCore -lclangCrossTU -lclangIndex -lclangParse -lclangSerialization -lclangSema -lclangAnalysis -lclangEdit -lclangFormat -lclangToolingInclusions -lclangToolingCore -lclangRewrite -lclangASTMatchers -lclangAST -lclangLex -ldl -I /usr/lib/llvm-10/include/ -L/usr/lib/llvm-10/lib/

[Meinersbur]

-Polly, -lPollyPPCG and -lPollyISL are missing.

Where does the list of libraries come from? llvm-config?

[jpalus]

-Polly, -lPollyPPCG and -lPollyISL are missing.

Where does the list of libraries come from? llvm-config?

It appears that both opencl-clang and Mesa are constructing list of libclang* libs to be linked manually, see https://github.com/intel/opencl-clang/blob/master/CMakeLists.txt#L233 and relevant comment intel/opencl-clang#112 (comment). Could you please confirm that libPolly is now mandatory when linking against libclangCodeGen and therefore should be added to the list? Could you suggest a more robust way of obtaining proper libs?

[Meinersbur]

The Polly libraries are only dependencies when LLVM was compiled using LLVM_POLLY_LINK_INTO_TOOLS=ON, which now is the default if Polly is enabled.

An introspection with CMake's find_package(LLVM) would be the most robust way to find all required libraries. Alternatively, llvm-config an be asked for this information.

@​serge-sans-paille: I think LLVM__LINK_INTO_TOOLS current does not add plugin dependencies into the list returned by llvm-config -libs.

[jpalus]

An introspection with CMake's find_package(LLVM) would be the most robust
way to find all required libraries.

Do you mean some extra steps or just find_package call? Note that there is such step already in opencl-clang but I guess it does not add libPolly:

-- Looking for LLVM version 10.0.0
-- Using LLVMConfig.cmake in: /usr/lib64/cmake/llvm
-- Found LLVM 10.0.0

https://github.com/intel/opencl-clang/blob/master/CMakeLists.txt#L26

[serge-sans-paille]

@​serge-sans-paille: I think LLVM__LINK_INTO_TOOLS current does not add
plugin dependencies into the list returned by llvm-config -libs.

That's a bug! I'll have a look.

[serge-sans-paille]

@​serge-sans-paille: I think LLVM__LINK_INTO_TOOLS current does not add
plugin dependencies into the list returned by llvm-config -libs.

That's a bug! I'll have a look.

There's currently no mode in llvm-config to gather dependency information for clang (or lld) libs. And llvm-config is probably not the right place to do this (clang can be compiled in standalone mode). Should we add a clang-config tool ?

[andrewrk]

Please, can this be solved without requiring downstream projects to use CMake?

[andrewrk]

In ziglang/zig#4992 I attempted to change zig to use LLVMConfig.cmake and ClangConfig.cmake, however I ran into this problem, using apt.llvm.org:

"make[2]: *** No rule to make target '/usr/lib/llvm-10/lib/libPolly.a'

I just got the package sources
polly isn't referenced in any debian/*.install file

So there is definitely a packaging problem here

[jpalus]

In case anyone's interested I've submitted pull requests to both opencl-clang and Mesa. Not that I'm satisfied with them but I couldn't think of anything better. I really think libclangCodeGen's deps should be pulled from somewhere dynamically instead of every downstream project tracking and fixing undefined symbols for different LLVM build configurations. Also no idea why opencl-clang is fine with just -lPolly while Mesa requires both -lPolly and -lPollyISL (neither of them required Polly before).

intel/opencl-clang#117

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4511

@​Andrew: I'm not familiar apt.llvm.org but I suspect it might have something to do with:

Don't think it is used

rm -f $(DEB_INST)/usr/lib/llvm-$(LLVM_VERSION)/lib/libPolly*a

https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/8/debian/rules#L625

[andrewrk]

This has caused a lot of downstream frustrations, if you read starting here: ziglang/zig#4799 (comment) and continuing downwards.

Why is Polly special? Why can't it be a normal library like all the other ones?

[sylvestre]

I fixed the libpolly link on the apt.llvm.org side and triggered rebuilds
Should be live in a day or two
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/commit/2e5e0ec30a64decf5019cadd66003fa2fc00dced

[Meinersbur]

Why is Polly special? Why can't it be a normal library like all the other
ones?

In contrast to all other libraries, the Polly dependency is optional, depending on how LLVM was configured before compilation.

[jpalus]

Any chance to get rid of this hard symbol dependency on Polly that consists of single symbol? Ie calling dlsym(RTLD_DEFAULT, "getPollyPluginInfo") and if NULL is returned assume Polly is not available?

[tstellar]

Can someone summarize the status of this bug? Which commits are confirmed to fix the problem?

[Meinersbur]

I think what caused all this is the following change in D61446:

• Before: The tool's executables (opt.cpp, bugpoint.cpp, clang_cc1.cpp) depend on the Polly libraries.
• After: Clang's dependency moved into one of its libraries (clangCodeGen)

This means that every user of clangCodeGen new also needs to link Polly. This is no problem when using CMake's find_package, as CMake automatically has generated a ClangConfig.cmake containing:

set_target_properties(clangCodeGen PROPERTIES
  INTERFACE_LINK_LIBRARIES "[...];Polly"
)

hence using link_libraries(MyCompilerProject clangCodeGen) will automatically add -lPolly to the linker command line.

Projects not using CMake's find_package (e.g. because they are not using CMake at all) typically hardcodes all required libraries, due to the absence of clang-config (such as https://github.com/intel/opencl-clang/blob/master/CMakeLists.txt#L233).

Solutions:

1. Move the configuration-dependent of clangCodeGen to Polly back to the clang executable.
   This is not easy to do since the NewPM pass registration takes place in clangCodeGen and unlike the legacy PM, there is no global list of registered pass plugins. What I could imagine to do is to make EmitAssemblyHelper::EmitAssemblyWithNewPassManager call back into the clang executable such that it can register its custom passes.

2. Create clang-config and require all (non-CMake) users of the clang libraries to use it.

3. Some downstream projects link against 'all' LLVM libraries (such as https://github.com/intel/opencl-clang/blob/83837744ef076c1285ed2ee9349febc2208d3252/CMakeLists.txt#L266) obtained from AddLLVM.cmake or llvm-config --libs. This currently does not emit any statically linked plugin library such as Polly (presumably for the same reason it does not show the Clang libraries as well). We could make it emit statically linked pass plugins as well.

For opt and bugpoint, it's still only in executables (opt.cpp -> NewPMDriver.cpp; bugpoint.cpp -> bugpoint.cpp) as the pass registration takes place in the executables.

[serge-sans-paille]

Thanks Michael for the very nice summary. +1 for Solution 3 which looks like the lowest friction path.

[Meinersbur]

Would you prepare a patch for that? It means that llvm-config --libs should emit -lPolly -lPollyPPCG -lPollyISL (or the equivalent for statically linked pass plugins including Bye; order relative to other LLVM libraries might be important) and add_llvm_library/executable(LINK_COMPONENTS all).

[serge-sans-paille]

Sure, I'm on it.

[serge-sans-paille]

WIP patch here: https://reviews.llvm.org/D78192

[serge-sans-paille]

These patch combined:

https://reviews.llvm.org/D78192
https://reviews.llvm.org/D78358

seem to solve the issue, and provide correct llvm-config + cmake integration.

Tested on various setup (with / withou llvm_dylib, through cmake and llvm-config)

The reproducer provided by Sylvestre Ledru also compiles fine with

g++ foo.cpp -lclangBasic -lclangCodeGen -lclangDriver -lclangFrontend -lclangFrontendTool -lclangCodeGen -lclangRewriteFrontend -lclangARCMigrate -lclangStaticAnalyzerFrontend -lclangStaticAnalyzerCheckers -lclangStaticAnalyzerCore -lclangCrossTU -lclangIndex -lclangFrontend -lclangDriver -lclangParse -lclangSerialization -lclangSema -lclangAnalysis -lclangEdit -lclangFormat -lclangToolingInclusions -lclangToolingCore -lclangRewrite -lclangASTMatchers -lclangAST -lclangLex -lclangBasic `./bin/llvm-config --cxxflags --ldflags --libs all`

[serge-sans-paille]

Just commited 8f766e3 that should fix the issue.

[tstellar]

Merged: 5f510e5

[tstellar]

mentioned in issue #44654

[serge-sans-paille]

mentioned in issue llvm/llvm-bugzilla-archive#45817

[jpalus]

mentioned in issue llvm/llvm-bugzilla-archive#47583