search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.48k stars 11.77k forks source link

clang 10.0.0_rc1 standalone fails to build with polly #120

Closed Keruspe closed 1 year ago

Keruspe commented 4 years ago

When llvm is built with polly, clang now fails to build with undefined symbol: getPollyPluginInfo().

9.0.1 builds fine

zmodem commented 4 years ago

Can you please provide steps to reproduce, i.e. with what cmake invocations etc. did you build llvm, polly, and clang?

Keruspe commented 4 years ago

llvm + polly (polly symlinked into the tools directory) were built with:

cmake -DCMAKE_C_FLAGS:STRING=-march=native -pipe -O3 -fgnuc-version=6.5.0 -flto=thin -DCMAKE_CXX_FLAGS:STRING=-march=native -pipe -O3 -fgnuc-version=6.5.0 -flto=thin -DCMAKE_AR:PATH=x86_64-pc-linux-gnu-ar -DCMAKE_RANLIB:PATH=x86_64-pc-linux-gnu-ranlib -DCMAKE_NM:PATH=x86_64-pc-linux-gnu-nm -DCMAKE_C_COMPILER:PATH=x86_64-pc-linux-gnu-cc -DCMAKE_CXX_COMPILER:PATH=x86_64-pc-linux-gnu-c++ -DCMAKE_INSTALL_PREFIX:PATH=/usr/x86_64-pc-linux-gnu -DCMAKE_FIND_ROOT_PATH=/usr/x86_64-pc-linux-gnu -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM:STRING=NEVER -DCMAKE_SYSTEM_PREFIX_PATH:PATH=/usr/x86_64-pc-linux-gnu -DCMAKE_INSTALL_LIBDIR:STRING=lib -DCMAKE_INSTALL_DATAROOTDIR:PATH=/usr/share/ -DPYTHON_EXECUTABLE:PATH=/usr/x86_64-pc-linux-gnu/bin/python3.8 -DLLVM_MAIN_SRC_DIR=/var/tmp/paludis/build/dev-lang-llvm-10.0.0_rc1/work/llvm-project/llvm -DLLVM_LIT_ARGS:STRING=-sv -DLLVM_LINK_LLVM_DYLIB:BOOL=ON -DLLVM_ENABLE_RTTI:BOOL=ON -DLLVM_ENABLE_EH:BOOL=ON -DLLVM_ENABLE_PIC:BOOL=ON -DCMAKE_BUILD_TYPE=Release -DSUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG:BOOL=OFF -DLLVM_BINUTILS_INCDIR:STRING=/usr/x86_64-pc-linux-gnu/include/ -DLLVM_DEFAULT_TARGET_TRIPLE:STRING=x86_64-pc-linux-gnu -DLLVM_INCLUDE_TESTS:BOOL=TRUE -DLLVM_BUILD_LLVM_DYLIB:BOOL=TRUE -DCMAKE_INSTALL_PREFIX:STRING=/usr/x86_64-pc-linux-gnu/lib/llvm/10 -DCMAKE_INSTALL_MANDIR:STRING=/usr/x86_64-pc-linux-gnu/lib/llvm/10/share/man -DLLVM_INSTALL_UTILS:BOOL=TRUE -DLLVM_ENABLE_LIBPFM:BOOL=FALSE -DLLVM_ENABLE_LIBXML2:BOOL=TRUE -DLLVM_ENABLE_WERROR:BOOL=FALSE -DLLVM_ENABLE_ASSERTIONS:BOOL=FALSE -DLLVM_ENABLE_LIBEDIT:BOOL=TRUE -DLLVM_POLLY_LINK_INTO_TOOLS:BOOL=TRUE -DLLVM_TOOL_POLLY_BUILD:BOOL=TRUE -DLLVM_BUILD_TESTS:BOOL=OFF -DLLVM_ENABLE_EXPENSIVE_CHECKS:BOOL=OFF -GNinja -DPOLLY_BUNDLED_ISL:BOOL=ON -DPOLLY_ENABLE_GPGPU_CODEGEN:BOOL=OFF /var/tmp/paludis/build/dev-lang-llvm-10.0.0_rc1/work/llvm-project/llvm

Clang is built with

cmake -DCMAKE_C_FLAGS:STRING=-march=native -pipe -O3 -fgnuc-version=6.5.0 -flto=thin -DCMAKE_CXX_FLAGS:STRING=-march=native -pipe -O3 -fgnuc-version=6.5.0 -flto=thin -DCMAKE_AR:PATH=x86_64-pc-linux-gnu-ar -DCMAKE_RANLIB:PATH=x86_64-pc-linux-gnu-ranlib -DCMAKE_NM:PATH=x86_64-pc-linux-gnu-nm -DCMAKE_C_COMPILER:PATH=x86_64-pc-linux-gnu-cc -DCMAKE_CXX_COMPILER:PATH=x86_64-pc-linux-gnu-c++ -DCMAKE_INSTALL_PREFIX:PATH=/usr/x86_64-pc-linux-gnu -DCMAKE_FIND_ROOT_PATH=/usr/x86_64-pc-linux-gnu -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM:STRING=NEVER -DCMAKE_SYSTEM_PREFIX_PATH:PATH=/usr/x86_64-pc-linux-gnu -DCMAKE_INSTALL_LIBDIR:STRING=lib -DCMAKE_INSTALL_DATAROOTDIR:PATH=/usr/share/ -DPYTHON_EXECUTABLE:PATH=/usr/x86_64-pc-linux-gnu/bin/python3.8 -DLLVM_MAIN_SRC_DIR=/var/tmp/paludis/build/dev-lang-clang-10.0.0_rc1/work/llvm-project/llvm -DLLVM_LIT_ARGS:STRING=-sv -DLLVM_LINK_LLVM_DYLIB:BOOL=ON -DLLVM_ENABLE_RTTI:BOOL=ON -DLLVM_ENABLE_EH:BOOL=ON -DLLVM_ENABLE_PIC:BOOL=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_CONFIG:STRING=/usr/x86_64-pc-linux-gnu/lib/llvm/10/bin/llvm-config -DCMAKE_INSTALL_PREFIX:STRING=/usr/x86_64-pc-linux-gnu/lib/llvm/10 -DCMAKE_INSTALL_MANDIR:STRING=/usr/x86_64-pc-linux-gnu/lib/llvm/10/share/man -DSUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG:BOOL=NO -DCLANG_BUILD_TOOLS:BOOL=ON -DCLANG_ENABLE_ARCMT:BOOL=ON -DCLANG_ENABLE_CLANGD:BOOL=ON -DCLANG_ENABLE_PROTO_FUZZER:BOOL=OFF -DCLANG_ENABLE_STATIC_ANALYZER:BOOL=ON -DCLANG_INSTALL_SCANBUILD:BOOL=ON -DCLANG_INSTALL_SCANVIEW:BOOL=ON -DCLANG_LINK_CLANG_DYLIB:BOOL=ON -DCLANG_PLUGIN_SUPPORT:BOOL=ON -DCLANG_RESOURCE_DIR:STRING=../lib/clang/10 -DCLANG_INCLUDE_TESTS:BOOL=OFF -GNinja -DCLANG_DEFAULT_CXX_STDLIB:STRING=libc++ -DCLANG_DEFAULT_RTLIB:STRING=compiler-rt -DCLANG_DEFAULT_UNWINDLIB:STRING=libunwind /var/tmp/paludis/build/dev-lang-clang-10.0.0_rc1/work/llvm-project/clang
zmodem commented 4 years ago

I spent some time trying to reproduce this, but with no luck so far. Can you provide more detailed repro steps?

This is what I tried:

$ wget https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0-rc1/llvm-10.0.0rc1.src.tar.xz
$ wget https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0-rc1/polly-10.0.0rc1.src.tar.xz
$ wget https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0-rc1/clang-10.0.0rc1.src.tar.xz

$ mkdir llvm
$ tar xf llvm-10.0.0rc1.src.tar.xz --strip-components=1 -C llvm
$ mkdir llvm/tools/polly
$ tar xf polly-10.0.0rc1.src.tar.xz --strip-components=1 -C llvm/tools/polly/

$ mkdir build1 && cd build1
$ cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/inst -DLLVM_LINK_LLVM_DYLIB=ON -DLLVM_BUILD_LLVM_DYLIB=ON   -DDLLVM_POLLY_LINK_INTO_TOOLS=ON -DDLLVM_TOOL_POLLY_BUILD=ON -DDPOLLY_BUNDLED_ISL=ON ../llvm && ninja && ninja install
$ cd ..

$ mkdir clang
$ tar xf clang-10.0.0rc1.src.tar.xz --strip-components=1 -C clang

$ mkdir build2 && cd build2
$ cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/inst -DLLVM_LINK_LLVM_DYLIB=ON -DCLANG_LINK_CLANG_DYLIB=ON -DLLVM_MAIN_SRC_DIR=/tmp/foo/llvm -DLLVM_CONFIG=/tmp/inst/bin/llvm-config ../clang && ninja clang
Keruspe commented 4 years ago

I wonder if that could be because of ThinLTO. Will do some more testing and will try to give more pointers towards reproduction, but here the way to reproduce is basically

Will try to do that outside of the package manager in a temporary location.

Keruspe commented 4 years ago

First test:

I unpacked llvm, polly and clang as you did, but only built clang, using the same command as you, apart from llvm-config (using the system one instead, as llvm 10.0.0_rc1 with polly is installed system-wide here). Got the same error:

ld.lld: error: undefined symbol: getPollyPluginInfo()
>>> referenced by BackendUtil.cpp
>>>               lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/BackendUtil.cpp.o:((anonymous namespace)::EmitAssemblyHelper::EmitAssemblyWithNewPassManager(clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream> >))
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Fwiw, the getPollyPluginInfo() calls are from the included /usr/lib/llvm/10/include/llvm/Support/Extension.def which contains

//extension handlers
HANDLE_EXTENSION(Polly)
#undef HANDLE_EXTENSION
Keruspe commented 4 years ago

I think I know why you weren't able to reproduce. When I wanted to reproduce your full scenario, I just noticed that the three cmake params for polly start with -DD instead of just -D and are thus ignored

  Manually-specified variables were not used by the project:

    DLLVM_POLLY_LINK_INTO_TOOLS
    DLLVM_TOOL_POLLY_BUILD
    DPOLLY_BUNDLED_ISL
Keruspe commented 4 years ago

Yup, I confirm that with these typo fixed, I can reproduce the problem with your commands

Meinersbur commented 4 years ago

The change causing this is probably 24ab9b537e61b3fe5e6a1019492ff6530d82a3ee. I suspect that -DLLVM_POLLY_LINK_INTO_TOOLS=ON does not cause Polly to be linked into libLLVM, which clang links against. It's a bit unusual that you build clang separately from LLVM (But I think it's still supposed to work). Ping @serge-sans-paille.

Note that -DLLVM_ENABLE_PROJECTS=polly mechanism is newer than -DLLVM_TOOL_POLLY_BUILD=ON . Our primary bug tracker is https://bugs.llvm.org/.

Keruspe commented 4 years ago

Building clang separately is at least how all of the source-based distribution do it AFAIK.

Do you want me to resubmit the bug to bugs.llvm.org?

zmodem commented 4 years ago

Yup, I confirm that with these typo fixed, I can reproduce the problem with your commands

D'oh! Thanks for confirming.

Do you want me to resubmit the bug to bugs.llvm.org?

Yes, it would be good to have it there so we can track it as a release blocker. I've filed https://bugs.llvm.org/show_bug.cgi?id=44870

Do you have a bugzilla account so I can cc you?

Keruspe commented 4 years ago

I don't (yet)

zmodem commented 4 years ago

The change causing this is probably 24ab9b5.

Yes, it seems after this change (at 0d275431a3abc96fdee3e09afdc84e59df0e1d3b which has a build fix), it starts failing to link with

undefined reference to `getPollyPluginInfo()'

However, I'm also not able to build before that. For example at 24ab9b537e61b3fe5e6a1019492ff6530d82a3ee^1 my build fails with

undefined reference to `polly::initializePollyPasses(llvm::PassRegistry&)'

So maybe I'm still not holding the build quite right. My reproduction command was (from a monorepo checkout):

$ rm -rf /tmp/issue120 && mkdir /tmp/issue120 && cp -r llvm clang /tmp/issue120/ && cp -r polly /tmp/issue120/llvm/tools/ && ( cd /tmp/issue120 && mkdir build1 && cd build1 && cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/inst -DLLVM_LINK_LLVM_DYLIB=ON -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_POLLY_LINK_INTO_TOOLS=ON -DLLVM_TOOL_POLLY_BUILD=ON -DPOLLY_BUNDLED_ISL=ON ../llvm && ninja && ninja install && cd .. && mkdir build2 && cd build2 && cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/inst -DLLVM_LINK_LLVM_DYLIB=ON -DCLANG_LINK_CLANG_DYLIB=ON -DLLVM_MAIN_SRC_DIR=/tmp/issue120/llvm -DLLVM_CONFIG=/tmp/inst/bin/llvm-config ../clang && ninja clang )

Keruspe commented 4 years ago

For building 9.0.1, we use LLVM_POLLY_BUILD instead of LLVM_TOOL_POLLY_BUILD for llvm (option was renamed for 10), and we add WITH_POLLY=TRUE (which is gone in 10) and LINK_POLLY_INTO_TOOLS=TRUE (which was renamed in 10)

serge-sans-paille commented 4 years ago

I managed to reproduce the issue and found the actual problem: the extension mechanism is not aware of the DYLIB process, and linked-in extensions are not selected when selecting all components.

I'm currently trying a patch, if it works I'll submit it for review.

Meinersbur commented 4 years ago

The change causing this is probably 24ab9b5.

Yes, it seems after this change (at 0d27543 which has a build fix), it starts failing to link with

undefined reference to `getPollyPluginInfo()'

However, I'm also not able to build before that. For example at 24ab9b5^1 my build fails with

undefined reference to `polly::initializePollyPasses(llvm::PassRegistry&)'

This confirms my suspicion that libPolly is just not added into libLLVM.so (the dylib). Before 24ab9b5, the clang executable pulled-in Polly into the executable by calling initializePollyPasses. After 24ab9b5 it is pulled-in by the registered plugin (which registered getPollyPluginInfo as entry point). Try adding Polly to -DLLVM_DYLIB_COMPONENTS=. If that works, we can solve it my adding registered plugins to the all list (see llvm/cmake/modules/LLVM-Config.cmake).

serge-sans-paille commented 4 years ago

https://reviews.llvm.org/D74464 fixes the issue

serge-sans-paille commented 4 years ago

Closed by d21664cce1db8debe2528f36b1fbd2b8af9c9401

Keruspe commented 4 years ago

@serge-sans-paille I don't think that's quite enough.

When llvm 10 is installed system-wide and I try to build clang against it, the new added line tries to write to /usr/lib/llvm/10/include/llvm/Support/Extension.def.tmp which it's obviously not allowed to do. It should only create Extension.def when called from llvm, not when called from clang

serge-sans-paille commented 4 years ago

Correct! should be fixed by https://reviews.llvm.org/D74602

Keruspe commented 4 years ago

So, just retried the full scenario from above, but using 10.0.0rc2 + applying D74464 and D74602

I still get the initial error:

ld.lld: error: undefined symbol: getPollyPluginInfo()
>>> referenced by BackendUtil.cpp
>>>               lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/BackendUtil.cpp.o:((anonymous namespace)::EmitAssemblyHelper::EmitAssemblyWithNewPassManager(clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream> >))

Here is what I ran:

mkdir /tmp/foo
cd /tmp/foo

wget https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0-rc2/llvm-10.0.0rc2.src.tar.xz
wget https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0-rc2/polly-10.0.0rc2.src.tar.xz
wget https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0-rc2/clang-10.0.0rc2.src.tar.xz

mkdir llvm
tar xf llvm-10.0.0rc2.src.tar.xz --strip-components=1 -C llvm
mkdir llvm/tools/polly
tar xf polly-10.0.0rc2.src.tar.xz --strip-components=1 -C llvm/tools/polly/
mkdir clang
tar xf clang-10.0.0rc2.src.tar.xz --strip-components=1 -C clang

curl https://github.com/llvm/llvm-project/commit/d21664cce1db8debe2528f36b1fbd2b8af9c9401.patch | patch -p1
curl https://reviews.llvm.org/file/data/nrlsy72arffezufkqttt/PHID-FILE-lslvurprq7y74emgjb7w/D74602.diff | patch -p1

mkdir build1 && cd build1
cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/inst -DLLVM_LINK_LLVM_DYLIB=ON -DLLVM_BUILD_LLVM_DYLIB=ON   -DLLVM_POLLY_LINK_INTO_TOOLS=ON -DLLVM_TOOL_POLLY_BUILD=ON -DPOLLY_BUNDLED_ISL=ON ../llvm && ninja && ninja install
cd ..

mkdir build2 && cd build2
cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/inst -DLLVM_LINK_LLVM_DYLIB=ON -DCLANG_LINK_CLANG_DYLIB=ON -DLLVM_MAIN_SRC_DIR=/tmp/foo/llvm -DLLVM_CONFIG=/tmp/inst/bin/llvm-config ../clang && ninja clang
Keruspe commented 4 years ago

I guess this is because LLVM_COMPILE_EXTENSIONS doesn't contain Polly since add_llvm_pass_plugin from Polly isn't called in standalone clang?

Keruspe commented 4 years ago

So, obviously not the right fix, but adding set_property(GLOBAL APPEND PROPERTY LLVM_COMPILE_EXTENSIONS Polly) just before the call to process_llvm_pass_plugins(NO_GEN) allows me to pass -DLLVM_POLLY_LINK_INTO_TOOLS=ON to clang and the build then passes.

Maybe the final value of LLVM_COMPILE_EXTENSIONS should be retrieved somehow with llvm-config or whatever when building standalong clang?

serge-sans-paille commented 4 years ago

Maybe the final value of LLVM_COMPILE_EXTENSIONS should be retrieved somehow with llvm-config or whatever when building standalong clang?

I like that idea, yeah. Let me dig into it later one. I still commitedD74602 as a small step forward.

serge-sans-paille commented 4 years ago

Approach implemented in https://reviews.llvm.org/D74757

Keruspe commented 4 years ago

:+1: Testing in progress

Keruspe commented 4 years ago

Any chance of a backport of these patches to the llvm 10 branch?

serge-sans-paille commented 4 years ago

@Keruspe I think it's feasible, @zmodem ?

zmodem commented 4 years ago

I'm a little bit worried that this is landing so late in the process, and because it's non-trivial cmake changes it has the potential of breaking lots of people.

On the other hand, the use of Polly is very limited iiuc, so I'm not sure how bad the bug is.

What revisions would need to be ported, and what's your confidence level regarding their correctness?

serge-sans-paille commented 4 years ago

I'm a little bit worried that this is landing so late in the process, and because it's non-trivial cmake changes it has the potential of breaking lots of people.

So am I. On the other hand there's no regression on master branch as of now, so we already have some positive feedback :-)

Concerning the revision that need to be ported, I'd say

3a0f6e699bb6d96dc62dce6faef20ac26cf103fd 87dac7da68ea1e0adac78c59ef1891dcf9632b67

Concerning their correctness, I can't go further than: bots validation is OK, I tested several configuration locally and @Keruspe did too.

Plus this should only impact Polly users, so it should have limited impact.

zmodem commented 4 years ago

I looked at the changes again, and decided not to merge them.

It seems to me the risk (cmake breaking somehow) is too high for the reward (standalone clang builds with polly, which isn't widely used).

Maybe it can be a candidate for 10.0.1.

slacka commented 4 years ago

@serge-sans-paille Since @zmodem is averse to the risk of moving forward with the fixes, what are your thoughts on reverting 24ab9b5 for the 10.x series?

serge-sans-paille commented 4 years ago

@slacka / @zmodem : considering the multiple patch applied to the original commit, I'd understand if you totally delay it to 10.0.1 or 11.x

zmodem commented 4 years ago

@serge-sans-paille Since @zmodem is averse to the risk of moving forward with the fixes, what are your thoughts on reverting 24ab9b5 for the 10.x series?

Backing out this whole thing would be my preferred approach, but there seem to be too many changes on top to be able to do that cleanly.

llvmbot commented 2 years ago

@llvm/issue-subscribers-polly

arsenm commented 1 year ago

Old build issue on old release