terralang / terra

Terra is a low-level system programming language that is embedded in and meta-programmed by the Lua programming language.
terralang.org
Other
2.72k stars 201 forks source link

[WIP] Add LLVM 8 Support #408

Closed ProfFan closed 4 years ago

ProfFan commented 5 years ago

Test pass on my dev machine (LLVM 8.0.1, Arch Linux)

ProfFan commented 5 years ago

Related LLVM commits:

elliottslaughter commented 5 years ago

Thanks for starting the work on this! I made one note in the code that I think should fix the LLVM 7 build. When I build with LLVM 8 (build from source) I seem to be getting a lot of link errors like:

/home/elliott/sw/llvm-8/install/lib/libclangAnalysis.a(ExprMutationAnalyzer.cpp.o): In function `clang::ast_matchers::internal::matcher_hasDecayedType0Matcher::matches(clang::DecayedType const&, clang::ast_matchers::internal::ASTMatchFinder*, clang::ast_matchers::internal::BoundNodesTreeBuilder*) const':
ExprMutationAnalyzer.cpp:(.text._ZNK5clang12ast_matchers8internal30matcher_hasDecayedType0Matcher7matchesERKNS_11DecayedTypeEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE[_ZNK5clang12ast_matchers8internal30matcher_hasDecayedType0Matcher7matchesERKNS_11DecayedTypeEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE]+0x2c): undefined reference to `clang::ast_matchers::internal::DynTypedMatcher::matches(clang::ast_type_traits::DynTypedNode const&, clang::ast_matchers::internal::ASTMatchFinder*, clang::ast_matchers::internal::BoundNodesTreeBuilder*) const'
ProfFan commented 5 years ago

Strange, as I am on LLVM 8.0.1 now. Can you confirm that you are building with CMake and with dynamic linking?

elliottslaughter commented 5 years ago

Hm, I'm running:

CMAKE_PREFIX_PATH=$HOME/sw/llvm-8/install cmake .. -DTERRA_STATIC_LINK_LLVM=OFF -DCMAKE_INSTALL_PREFIX=$PWD/../install && make install -j40

Am I missing something?

LLVM itself was built with:

cmake ../llvm-8.0.1.src -DCMAKE_INSTALL_PREFIX=$PWD/../install -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_TERMINFO=OFF -DLLVM_ENABLE_LIBEDIT=OFF -DLLVM_ENABLE_ZLIB=OFF -DLLVM_ENABLE_ASSERTIONS=OFF -G Ninja
ProfFan commented 5 years ago

@elliottslaughter I am using pretty much the same commands as you, though I am not sure about the LLVM side, as I am using the packaged version in Arch Linux.

https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/llvm

  cmake .. -G Ninja \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=/usr \
    -DLLVM_HOST_TRIPLE=$CHOST \
    -DLLVM_BUILD_LLVM_DYLIB=ON \
    -DLLVM_LINK_LLVM_DYLIB=ON \
    -DLLVM_INSTALL_UTILS=ON \
    -DLLVM_ENABLE_RTTI=ON \
    -DLLVM_ENABLE_FFI=ON \
    -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=AVR \
    -DLLVM_BUILD_TESTS=ON \
    -DLLVM_BUILD_DOCS=ON \
    -DLLVM_ENABLE_SPHINX=ON \
    -DLLVM_ENABLE_DOXYGEN=OFF \
    -DSPHINX_WARNINGS_AS_ERRORS=OFF \
    -DFFI_INCLUDE_DIR=$(pkg-config --variable=includedir libffi) \
    -DLLVM_BINUTILS_INCDIR=/usr/include
elliottslaughter commented 5 years ago

Ok, I got it to build. The critical flags seem to have been -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON (or possibly just one of those, I didn't bother to check). I guess this is fine for the first pass but I'd like to understand what is wrong in the static build since I'd really prefer to not to require a non-standard build of LLVM.

All tests passed, including CUDA.

ProfFan commented 5 years ago

Yeah me too but I am too busy for that :(

elliottslaughter commented 5 years ago

Ok, thanks for taking things this far. I pushed a change to test LLVM 8 in Travis, if that passes I may pull this just so we don't risk bitrot, but either I or someone else will need to find the time to dig into what's going on with the static build.

ProfFan commented 5 years ago

A git bisect could be very useful I think.

elliottslaughter commented 5 years ago

Ok, after rebuilding LLVM from source ~15 times today (thank goodness for 40-core machines), bisect arrived at the following commit:

e9192f83894cc0e09148515fb8cc59aeddc077c3 is the first bad commit
commit e9192f83894cc0e09148515fb8cc59aeddc077c3
Author: Shuai Wang <shuaiwang@google.com>
Date:   Tue Sep 11 21:13:20 2018 +0000

    [analyzer] Add ExprMutationAnalyzer

    Summary:
    This is 1/2 of moving ExprMutationAnalyzer from clangtidy to
    clang/Analysis.
    This diff along simply copies the ExprMutationAnalyzer over with trivial
    modifications (e.g. include path, namespace)
    2/2 will migrate existing usage of ExprMutationAnalyzer and remove the
    original copy inside clangtidy.

    Reviewers: george.karpenkov

    Subscribers: mgorny, xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits, JonasToth

    Differential Revision: https://reviews.llvm.org/D51948

    llvm-svn: 341994

:040000 040000 775f79c821d7182d3ab2516b97c80ffa4803041b e06db7b5f2ccdeec3f5a66a6db490b6ae983805b M  clang

I haven't actually had an opportunity to look at what is inside it yet.

ProfFan commented 5 years ago

Oh that is one hell of a tedious job! I looked a bit into it, can't find any obvious issues :(

ProfFan commented 5 years ago

Could be some lazy-binding issue, need further investigation

ErikMcClure commented 4 years ago

It is possible LLVM is not properly exporting functions, but it was previously masked by them being in header files, since this seems to involve a refactor.

I have not been able to look into this, but I would like someone to figure out the static build problems because I do not want to rely on dynamic builds of LLVM on linux.

elliottslaughter commented 4 years ago

I agree, but (as you can see) haven't had any time to look into this. If anyone has time to do so, that would be great, otherwise it's sitting in my queue of things to do.

The other option is to skip ahead to LLVM 9 and hope they've fixed this in the mean time.

ErikMcClure commented 4 years ago

I recently moved my project to LLVM 9, so I might attempt this if I get time. I will make a new pull request if I do.

elliottslaughter commented 4 years ago

I did an initial LLVM 9 attempt at https://github.com/elliottslaughter/terra/tree/llvm-9, but I've probably done something stupid since it doesn't link in either dynamic or static configuration.

elliottslaughter commented 4 years ago

Looks like it was just a new Clang sub-module. Should be fixed now, PR at #419. Static linking seems to work too.

Thanks again to @ProfFan for the hard work! I will probably merge via #419 (which also includes all of this PR) once it passes.

elliottslaughter commented 4 years ago

What do you know, my fix for LLVM 9 actually works for LLVM 8 too. So I think we'll get both 8 and 9 support soon now.

elliottslaughter commented 4 years ago

I merged this via #419. Thanks again to everyone for your help!