search

crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.47k stars 1.62k forks source link

Support LLVM6 #5556

Closed ysbaddaden closed 6 years ago

ysbaddaden commented 6 years ago

LLVM 6.0 was branched, we may start investigating what got broken, and more importantly: what got added to the C API (surprise).

asterite commented 6 years ago

Surprise?? 😮

ysbaddaden commented 6 years ago

From an initial attempt (ubuntu trusty, using prebuilt https://apt.llvm.org packages), I got a few hiccups. For example missing support for GCC 4.8.5 (no option -Wdate-time), and a missing symbol (LLVMInitializeInstCombine) probably due to the packages, since we never call the function directly, and fixed with the --link-static flag, such as:

$ make clean crystal \
  LLVM_CONFIG="llvm-config-6.0 --link-static" \
  CXX=clang++-5.0 CC=clang-5.0

I then got actual compatibility issues:

/home/github/crystal/src/llvm/ext/llvm_ext.o: In function `LLVMDIBuilderCreateCompileUnit':
src/llvm/ext/llvm_ext.cc:(.text.LLVMDIBuilderCreateCompileUnit+0x12f): undefined reference to `llvm::DIBuilder::createCompileUnit(unsigned int, llvm::DIFile*, llvm::StringRef, bool, llvm::StringRef, unsigned int, llvm::StringRef, llvm::DICompileUnit::DebugEmissionKind, unsigned long, bool, bool, bool)'
/home/github/crystal/src/llvm/ext/llvm_ext.o: In function `LLVMDIBuilderCreateFunction':
src/llvm/ext/llvm_ext.cc:(.text.LLVMDIBuilderCreateFunction+0xf1): undefined reference to `llvm::DIBuilder::createFunction(llvm::DIScope*, llvm::StringRef, llvm::StringRef, llvm::DIFile*, unsigned int, llvm::DISubroutineType*, bool, bool, unsigned int, llvm::DINode::DIFlags, bool, llvm::MDTupleTypedArrayWrapper<llvm::DITemplateParameter>, llvm::DISubprogram*, llvm::MDTupleTypedArrayWrapper<llvm::DIType>)'
/home/github/crystal/src/llvm/ext/llvm_ext.o: In function `LLVMDIBuilderCreatePointerType':
src/llvm/ext/llvm_ext.cc:(.text.LLVMDIBuilderCreatePointerType+0x52): undefined reference to `llvm::DIBuilder::createPointerType(llvm::DIType*, unsigned long, unsigned int, llvm::Optional<unsigned int>, llvm::StringRef)'
/home/github/crystal/src/llvm/ext/llvm_ext.o: In function `llvm::IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>::CreateAtomicCmpXchg(llvm::Value*, llvm::Value*, llvm::Value*, llvm::AtomicOrdering, llvm::AtomicOrdering, unsigned char)':
src/llvm/ext/llvm_ext.cc:(.text._ZN4llvm9IRBuilderINS_14ConstantFolderENS_24IRBuilderDefaultInserterEE19CreateAtomicCmpXchgEPNS_5ValueES5_S5_NS_14AtomicOrderingES6_h[_ZN4llvm9IRBuilderINS_14ConstantFolderENS_24IRBuilderDefaultInserterEE19CreateAtomicCmpXchgEPNS_5ValueES5_S5_NS_14AtomicOrderingES6_h]+0x60): undefined reference to `llvm::AtomicCmpXchgInst::AtomicCmpXchgInst(llvm::Value*, llvm::Value*, llvm::Value*, llvm::AtomicOrdering, llvm::AtomicOrdering, unsigned char, llvm::Instruction*)'
/home/github/crystal/src/llvm/ext/llvm_ext.o: In function `LLVMWriteBitcodeWithSummaryToFile':
src/llvm/ext/llvm_ext.cc:(.text.LLVMWriteBitcodeWithSummaryToFile+0xa9): undefined reference to `llvm::WriteBitcodeToFile(llvm::Module const*, llvm::raw_ostream&, bool, llvm::ModuleSummaryIndex const*, bool, std::array<unsigned int, 5ul>*)'

No fun: our custom C bindings for C++ functions are broken. Namely DebugInfo, LTO (I think) and Atomics. Sigh.

Surprise: llvm-c/DebugInfo.h can be found in official C bindings. It's experimental and unstable, but there is some initial, official, C bindings for DIBuilder. Woot! See https://github.com/llvm-mirror/llvm/blob/release_60/include/llvm-c/DebugInfo.h which provides

asterite commented 6 years ago

Ooooh... I thought the surprise was going to be fun :-(

But well, hopefully little by little they will be completing the C API so that we don't need those ugly wrappers anymore.

ysbaddaden commented 6 years ago

Sorry, it's a surprise for LLVM maintainers that don't have to maintain C++ anymore!

A better one would have been: compiles just fine :grin:

RX14 commented 6 years ago

So LLVM6 has been released, we probably want to start seriously working on this before we're caught out by early adopter distributions like archlinux wanting to upgrade.

RX14 commented 6 years ago

Also:

The Debugify pass was added to opt to facilitate testing of debug info preservation. This pass attaches synthetic DILocations and DIVariables to the instructions in a Module. The CheckDebugify pass determines how much of the metadata is lost.

Hopefully this should help future LLVM releases improve on the pretty bad debug information of crystal binaries, since IIRC the problem is largely LLVM instead of crystal.

ysbaddaden commented 6 years ago

I'm feeling lazy this time. If someone wants to take a stab at this.

There are mostly C++ API changes but they seem to touch all the extensions we wrote (debug info, atomics, ...), and the introduction of a very minimal debug info support in the C API (i.e. declare file:line:column locations).

RX14 commented 6 years ago

I'm super confused: LLVMInitializeInstCombine is still in the llvm6 sourcecode and headers, plus apparently I can compile llvm_ext.cc with no changes?

RX14 commented 6 years ago

OK, LLVMInitializeInstCombine is a bug: https://reviews.llvm.org/D44140. Will be fixed for 6.0.1

RX14 commented 6 years ago

LLVM 6 with the LLVMInitializeInstCombine works without any changes from our side:

$ bin/crystal -v
Using compiled compiler at `.build/crystal'
Crystal 0.24.2+184 [863f301cf] (2018-03-16)

LLVM: 6.0.0
Default target: x86_64-pc-linux-gnu

The errors @ysbaddaden saw must have been fixed between the prerelease and release.

anatol commented 6 years ago

I am building crystal 0.24.2 and I see this weird compilation issue

Using /usr/bin/llvm-config [version=6.0.0]
g++ -c -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt  -o src/llvm/ext/llvm_ext.o src/llvm/ext/llvm_ext.cc `/usr/bin/llvm-config --cxxflags`
cc -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -fPIC  -D_FORTIFY_SOURCE=2  -c -o src/ext/sigfault.o src/ext/sigfault.c
ar -rcs src/ext/libcrystal.a src/ext/sigfault.o
./bin/crystal build --release --no-debug -D i_know_what_im_doing -o .build/crystal src/compiler/crystal.cr -D without_openssl -D without_zlib
Using /usr/bin/llvm-config [version=6.0.0]
./bin/crystal docs src/docs_main.cr
Error in src/docs_main.cr:8: while requiring "./csv"

require "./csv"
^

in src/csv.cr:422: while requiring "./csv/**"

require "./csv/**"
^

in src/csv/builder.cr:1: while requiring "csv"

require "csv"
^

in share/crystal/src/csv.cr:59: already initialized constant CSV::DEFAULT_SEPARATOR

  DEFAULT_SEPARATOR  = ','
  ^~~~~~~~~~~~~~~~~

make: *** [Makefile:93: docs] Error 1
j8r commented 6 years ago

already initialized constant CSV::DEFAULT_SEPARATOR - not linked to LLVM6. @anatol, you should report this to a separated issue.

ghost commented 6 years ago

iirc llvm6 has initial support for coroutines and crystal has them as well? for reference zig

and the homebrew bottle could be updated because llvm was bumped to v6, v5 is now a versioned formula and installed additionally for anyone upgrading homebrew lately (after llvm version bump, because before the bump llvm was installed as an unversioned formula whoich was v5) crystal-lang.rb

RX14 commented 6 years ago

@anatol Looks like an issue with CRYSTAL_PATH. How did you work around libLLVM-5.0.so being required by the old compiler and libLLVM-6.0.so being required by the new compiler? The error is likely related to that workaround. A PKGBUILD or something reproducible would help.

EDIT: it definitely is. See

in src/csv.cr

and

in share/crystal/src/csv.cr
anatol commented 6 years ago

I see this DEFAULT_SEPARATOR issue with LLVM5, so indeed it might be a config/build issue. Let's move this discussion to #5835

foutrelis commented 6 years ago

How did you work around libLLVM-5.0.so being required by the old compiler and libLLVM-6.0.so being required by the new compiler?

Copying libLLVM-5.0.so into the chroot before the build started worked out OK. :P

(I did the rebuild for LLVM 6 a couple days after the LLVMInitializeInstCombine fix landed.)

joshuapinter commented 5 years ago

Likely not the right place for this but it's related.

Trying to build crystal on Ubuntu 18.04 so I can do some additional testing for PR https://github.com/crystal-lang/crystal/pull/7507 and getting the following error: 

$ make
Using /usr/bin/llvm-config-6.0 [version=6.0.0]
CRYSTAL_CONFIG_PATH="/home/joshuapinter/Development/crystal/src" CRYSTAL_CONFIG_BUILD_COMMIT="88a238e2d" ./bin/crystal build -D preview_overflow -D compiler_rt  -o .build/crystal src/compiler/crystal.cr -D without_openssl -D without_zlib
L-L-V-M-5858P-assR-egistry.o: In function `initialize_inst_combine':
/home/joshuapinter/Development/crystal/src/llvm/pass_registry.cr:11: undefined reference to `LLVMInitializeInstCombine'
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc "${@}" -o '/home/joshuapinter/Development/crystal/.build/crystal'  -rdynamic  /home/joshuapinter/Development/crystal/src/llvm/ext/llvm_ext.o `/usr/bin/llvm-config-6.0 --libs --system-libs --ldflags 2> /dev/null` -lstdc++ -lpcre -lm -lgc -lpthread /home/joshuapinter/Development/crystal/src/ext/libcrystal.a -levent -lrt -ldl -L/usr/lib -L/usr/local/lib`
Makefile:122: recipe for target '.build/crystal' failed
make: *** [.build/crystal] Error 1

Version

$ crystal --version
Crystal 0.27.2 [60760a546] (2019-02-05)

LLVM: 4.0.0
Default target: x86_64-unknown-linux-gnu

It looks like I've got LLVM 4.0.0 installed but using llvm-config-6.0.

Sorry, a little out of my element here. Any help would be great.

straight-shoota commented 5 years ago

@joshuapinter Please ask this question in the forum or on Gitter/IRC.

joshuapinter commented 5 years ago

@straight-shoota Will do, thanks!

girng commented 5 years ago

@straight-shoota This is a very nasty bug, not a question for the forum (where developers rarely check). It needs to be fixed and addressed by core developers, we can't even compile Crystal when we follow the guide. I made an official issue about it and I sincerely hope it can be fixed permanently, not randomly show up a year later.