Support LLVM 3.9 #3439

ysbaddaden · 2016-10-19T00:17:57Z

Breaking changes (introduced by LLVM 3.9):

LLVMAddTargetData was removed (deprecated since 3.7).
LLVMGetTargetMachineData was removed (replaced by LLVMCreateTargetDataLayout).

Also:

drop custom data layouts to favor the LLVM default ones;
kept support for 3.5, 3.6 and 3.8 along with 3.9;
nice LLVM version macros (copied from Rust);
select llvm-config to use with make LLVM_CONFIG=llvm-config-3.6 to select a specific LLVM version among the many that are installed.

Errors:

#crashing compiler specs (not reproducible when run directly)

JIT messed with the unwind table. See 7517056.
--debug fails with Module validation failed: subprogram definitions must have a compile unit

We used to generate the DICompileUnit after generating the IR code, but LLVM reverses the DICompileUnit and DISubprogram relationships. It now refers to the CU from the SP. We thus have to genrate the CU as soon as possible (i.e. immediately after instanciating the DIBuilder).
--debug fail with inlinable function call in a function with debug info must have a !dbg location

closes #3237

Sija · 2016-10-19T00:23:39Z

src/llvm/function_pass_manager.cr

-  end
+  {% if LibLLVM::IS_35 || LibLLVM::IS_36 %}
+    def add_target_data(target_data)
+      puts "LLVMAddTargetData"


debug leftovers? btw, there's another one in module_pass_manager.cr.

Good catch. I just removed them.

bcardiff · 2016-10-19T01:42:33Z

Are the default data layouts of llvm <= 3.8 the same as the custom data layouts we used to have? I am not sure if they weren't used because we didn't know about them or if there were some issues with them.

ysbaddaden · 2016-10-19T08:47:07Z

The custom data layout were added in 5666375 which states « "Set llvm module data layout. Apparently this helps llvm do more optimizations". We still set the data layout on the module, so LLVM should optimize out, but instead of harcoding them, we let LLVM create the best one for the target triple (not just the arch). I compiled Crystal in release mode and ran specs for each LLVM version on Linux64 and they passed.

But I have some segfaults when running the compiler specs against LLVM 3.9 😢

ysbaddaden · 2016-10-19T09:27:47Z

The segfault always come down to the following backtrace (gdb).

Program received signal SIGSEGV, Segmentation fault.
classify_object_over_fdes (ob=ob@entry=0xe18f920, this_fde=0x7ffff7ff5000)
    at ../../../src/libgcc/unwind-dw2-fde.c:613
613     ../../../src/libgcc/unwind-dw2-fde.c: Aucun fichier ou dossier de ce type.
(gdb) bt
#0  classify_object_over_fdes (ob=ob@entry=0xe18f920, this_fde=0x7ffff7ff5000)
    at ../../../src/libgcc/unwind-dw2-fde.c:613
#1  0x00007ffff327091d in init_object (ob=0xe18f920) at ../../../src/libgcc/unwind-dw2-fde.c:749
#2  search_object (ob=ob@entry=0xe18f920, pc=pc@entry=0x7ffff326f6a7 <_Unwind_Backtrace+55>)
    at ../../../src/libgcc/unwind-dw2-fde.c:961
#3  0x00007ffff32711c2 in _Unwind_Find_registered_FDE (bases=0x7fffffffb6b8, 
    pc=0x7ffff326f6a7 <_Unwind_Backtrace+55>) at ../../../src/libgcc/unwind-dw2-fde.c:1025
#4  _Unwind_Find_FDE (pc=0x7ffff326f6a7 <_Unwind_Backtrace+55>, bases=bases@entry=0x7fffffffb6b8)
    at ../../../src/libgcc/unwind-dw2-fde-dip.c:450
#5  0x00007ffff326e507 in uw_frame_state_for (context=context@entry=0x7fffffffb610, fs=fs@entry=0x7fffffffb460)
    at ../../../src/libgcc/unwind-dw2.c:1245
#6  0x00007ffff326ee65 in uw_init_context_1 (context=context@entry=0x7fffffffb610, 
    outer_cfa=outer_cfa@entry=0x7fffffffb8c0, outer_ra=0x7df5b7 <*CallStack::unwind:Array(Pointer(Void))+87>)
    at ../../../src/libgcc/unwind-dw2.c:1566
#7  0x00007ffff326f6a8 in _Unwind_Backtrace (
    trace=0x7328b0 <~procProc(Pointer(Void), Pointer(Void), LibUnwind::ReasonCode)@./src/exception.cr:45>, 
    trace_argument=0x6776380) at ../../../src/libgcc/unwind.inc:283
#8  0x00000000007df5b7 in *CallStack::unwind:Array(Pointer(Void)) ()
#9  0x00000000007df54a in *CallStack#initialize:Array(Pointer(Void)) ()
#10 0x00000000007df51a in *CallStack::new:CallStack ()
#11 0x0000000000739108 in *raise<Crystal::TypeException>:NoReturn ()

If I copy paste the compiler samples from crashing specs, to run them directly with bin/crystal (LLVM 3.9, optimized), they do pass.

Some crashing specs I identified (there are more):

spec/compiler/codegen/c_struct_spec.cr: "automatically converts by invoking to_unsafe" (shouldn't raise)
spec/compiler/codegen/c_union_spec.cr: "automatically converts by invoking to_unsafe" (shouldn't raise)
a/spec/compiler/codegen/macro_spec.cr: "errors if dynamic constant assignment after macro expansion"

asterite · 2016-10-19T13:17:37Z

@ysbaddaden SWEET!!

When you say "Breaking changes", do you mean compiling against LLVM < 3.9 will stop working? At least travis seems happy... did you maybe mean there are some breaking changes in LLVM that you needed to handle, but no breaking changes will happen in Crystal? I'm just a bit worried about that because we are still using LLVM 3.6 for the official releases, and merging this might prevent us from releasing until we upgrade our omnibus build to LLVM 3.9.

ysbaddaden · 2016-10-19T13:59:21Z

@asterite don't worry, I merely listed the breaking changes introduced by LLVM 3.9 that were fixed. LLVM 3.5, 3.6 and 3.8 are still supported and working (it compiles and specs are passing). This PR only fixes compilation against 3.9, and can be merged already. Thought some other changes in LLVM 3.9 introduced bugs (see pull request description), so a Crystal binary against LLVM 3.9, despite working, isn't release-grade, yet.

ysbaddaden · 2016-10-19T16:48:13Z

I pushed the fix for --debug. I'm clueless about the failing compiler specs, thought...

ysbaddaden · 2016-10-20T15:07:57Z

I pushed a second fix for --debug by adding some debug locations to generated Crystal::Call AST in various places of the Semantic analyzer. Some expansions weren't always easy to spot :-)

Now I can build crystal and all_spec in debug mode (as well as some other projects).

EDIT: I messed history. I'm fixing that (thanks git reflog).

- LLVMGetTargetMachineDatawas replaced with LLVMCreateTargetDataLayout. - LLVMSetDataLayout was replaced by LLVMSetModuleDataLayout. This patch drops the custom data layouts. We now use the ones provided by LLVM that match the target triple.

…e unit) In LLVM 3.9 DISubprogram references the DICompileUnit directly, we thus need to generate the DICompileUnit before we generate LLVM IR code for the rest of the module (not after), otherwise the DISubprogram ends up with no DICompileUnit and LLVM fails to compile. This patch also enables DWARF versions above v2 for platforms that do support these versions (i.e. everything but OS X and Android). It also defines the "Debug Info Version" that matches the LLVM release.

Some syntax sugar expansions didn't set the location for some generated `Crystal::Call` nodes, which is rejected by LLVM 3.9 when the parent definition has a `!dbg` location itself.

ilovezfs · 2016-10-21T01:21:32Z

@ysbaddaden @asterite just a heads up that in Homebrew I've now upgraded the llvm formula to 3.9 and alongside that shipped crystal-lang with this PR applied as a patch. So a new release tag with this PR or equivalent merged would be much appreciated as soon as possible so that we can get rid of the patching and distribute an unaltered upstream release once again.

asterite · 2016-10-21T09:03:40Z

@ilovezfs Thank you! But why can't Crystal still depend on the llvm38 or llvm36 formulas? I see no need to upgrade to the latest version of llvm.

ilovezfs · 2016-10-21T09:06:10Z

Dependencies on anything in homebrew/versions are not allowed in homebrew/core.

ysbaddaden · 2016-10-21T09:28:41Z

This is harsh. Each new minor LLVM is tricky or plain hard to upgrade to. It would be fantastic to have a few LLVM releases in homebrew/core.

The patch proposed here is working but barely tested. The segfault in the compiler spec is worrisome (at best), and leads to the impossibility to hack the compiler with LLVM 3.9 for the time being...

ilovezfs · 2016-10-21T09:35:39Z

Can you not build crystal-lang with Xcode 8 alone?

asterite · 2016-10-21T10:16:31Z

I'm getting this when compiling against LLVM 3.9 on Mac:

Undefined symbols for architecture x86_64:
  "llvm::SymbolTableListTraits<llvm::Instruction, llvm::BasicBlock>::addNodeToList(llvm::Instruction*)", referenced from:
      llvm::AtomicCmpXchgInst* llvm::IRBuilder<true, llvm::ConstantFolder, llvm::IRBuilderDefaultInserter<true> >::Insert<llvm::AtomicCmpXchgInst>(llvm::AtomicCmpXchgInst*, llvm::Twine const&) const in llvm_ext.o
  "llvm::DIRef<llvm::DIType>::DIRef(llvm::Metadata const*)", referenced from:
      _LLVMDIBuilderCreateLocalVariable in llvm_ext.o
  "llvm::MDNode::getTemporary(llvm::LLVMContext&, llvm::ArrayRef<llvm::Metadata*>)", referenced from:
      _LLVMTemporaryMDNode in llvm_ext.o
  "llvm::DebugLoc::get(unsigned int, unsigned int, llvm::MDNode*, llvm::MDNode*)", referenced from:
      _LLVMSetCurrentDebugLocation2 in llvm_ext.o
  "llvm::DIBuilder::insertDeclare(llvm::Value*, llvm::DIVariable, llvm::DIExpression, llvm::BasicBlock*)", referenced from:
      _LLVMDIBuilderInsertDeclareAtEnd in llvm_ext.o
  "llvm::DIBuilder::createFunction(llvm::DIDescriptor, llvm::StringRef, llvm::StringRef, llvm::DIFile, unsigned int, llvm::DICompositeType, bool, bool, unsigned int, unsigned int, bool, llvm::Function*, llvm::MDNode*, llvm::MDNode*)", referenced from:
      _LLVMDIBuilderCreateFunction in llvm_ext.o
  "llvm::DIBuilder::createMemberType(llvm::DIDescriptor, llvm::StringRef, llvm::DIFile, unsigned int, unsigned long long, unsigned long long, unsigned long long, unsigned int, llvm::DIType)", referenced from:
      _LLVMDIBuilderCreateMemberType in llvm_ext.o
  "llvm::DIBuilder::createStructType(llvm::DIDescriptor, llvm::StringRef, llvm::DIFile, unsigned int, unsigned long long, unsigned long long, unsigned int, llvm::DIType, llvm::DITypedArray<llvm::DIDescriptor>, unsigned int, llvm::DIType, llvm::StringRef)", referenced from:
      _LLVMDIBuilderCreateStructType in llvm_ext.o
  "llvm::DIBuilder::createCompileUnit(unsigned int, llvm::StringRef, llvm::StringRef, llvm::StringRef, bool, llvm::StringRef, unsigned int, llvm::StringRef, llvm::DIBuilder::DebugEmissionKind, bool)", referenced from:
      _LLVMDIBuilderCreateCompileUnit in llvm_ext.o
  "llvm::DIBuilder::createPointerType(llvm::DIType, unsigned long long, unsigned long long, llvm::StringRef)", referenced from:
      _LLVMDIBuilderCreatePointerType in llvm_ext.o
  "llvm::DIBuilder::createLexicalBlock(llvm::DIDescriptor, llvm::DIFile, unsigned int, unsigned int)", referenced from:
      _LLVMDIBuilderCreateLexicalBlock in llvm_ext.o
  "llvm::DIBuilder::createLocalVariable(unsigned int, llvm::DIDescriptor, llvm::StringRef, llvm::DIFile, unsigned int, llvm::DIRef<llvm::DIType>, bool, unsigned int, unsigned int)", referenced from:
      _LLVMDIBuilderCreateLocalVariable in llvm_ext.o
  "llvm::DIBuilder::createSubroutineType(llvm::DIFile, llvm::DITypedArray<llvm::DIRef<llvm::DIType> >, unsigned int)", referenced from:
      _LLVMDIBuilderCreateSubroutineType in llvm_ext.o
  "llvm::DIBuilder::createEnumerationType(llvm::DIDescriptor, llvm::StringRef, llvm::DIFile, unsigned int, unsigned long long, unsigned long long, llvm::DITypedArray<llvm::DIDescriptor>, llvm::DIType, llvm::StringRef)", referenced from:
      _LLVMDIBuilderCreateEnumerationType in llvm_ext.o
  "llvm::DIDescriptor::isType() const", referenced from:
      _LLVMDIBuilderCreateLocalVariable in llvm_ext.o
  "llvm::DIScope::getRef() const", referenced from:
      _LLVMDIBuilderCreateLocalVariable in llvm_ext.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Nothing to worry about because I can still use an older LLVM, and it's probably something simple to fix because it worked for homebrew. @ysbaddaden Any idea?

asterite · 2016-10-21T10:21:50Z

Ah, ignore the above, I had llvm-config-3.6 in my path

ilovezfs · 2016-10-21T10:33:14Z

@asterite @ysbaddaden Homebrew/homebrew-core#6139 I've moved the patch into a stable-do block since it's no longer applicable to HEAD of master now that it's been merged here.

This seems to fix an issue with unwind stopping to work well after using a JIT without disposing it. See #3439

asterite · 2016-10-21T11:52:12Z

@ysbaddaden I just pushed a fix to the crashing-specs problem. Could you test that it works for you too? :-)

I'm not 100% sure about the fix but it works, though I'm 99% sure it has something to do with LLVM's MCJIT. At least in the past it brought us some issues, and that's one of the reasons why we don't use it for specs that use exceptions, so maybe there are still some bugs in it (on LLVM's side).

ysbaddaden · 2016-10-21T12:59:55Z

@asterite works for me! thanks!

Argh, I just got one more inlinable function call in a function with debug info must have a !dbg location when trying bin/crystal eval --release --debug 'puts "hello"'

asterite · 2016-10-21T13:11:16Z

@ysbaddaden What is this error? Can we maybe skip debug info for calls that don't have a location, or is this required in LLVM 3.9?

I found this error is raised for GC::add_finalizer, which is added automatically in a class' constructor. But this constructor could be the empty constructor which don't have a real location. Well, we could use the type's location...

asterite · 2016-10-21T13:14:15Z

@ysbaddaden I'll commit a fix for this soon, seems that using a type's location fixes it. There are a couple more calls that the codegen generates without a Call node, __crystal_raise and __crystal_get_exception, they are easy too.

asterite · 2016-10-21T13:28:53Z

@ysbaddaden Try now ;-)

ysbaddaden · 2016-10-21T13:37:06Z

@asterite thanks, it passes!

will · 2016-10-22T17:47:25Z

I saw @ysbaddaden's request for people to test. Tested head crystal with llvm 3.9 on crystal-pg specs and everything seems good.

ysbaddaden · 2016-10-22T21:42:00Z

Thanks for testing. Overall is seems good. Maybe --debug still has a few hidden quirks.

Sija reviewed Oct 19, 2016

View reviewed changes

ysbaddaden force-pushed the fix-llvm-3.9 branch from b364a52 to 00dcbcc Compare October 19, 2016 00:35

ysbaddaden force-pushed the fix-llvm-3.9 branch from 00dcbcc to e874df8 Compare October 19, 2016 10:33

ysbaddaden force-pushed the fix-llvm-3.9 branch from f844466 to b69dcad Compare October 19, 2016 19:29

ilovezfs mentioned this pull request Oct 20, 2016

llvm 3.9.0 Homebrew/homebrew-core#4579

Closed

6 tasks

ysbaddaden force-pushed the fix-llvm-3.9 branch from b69dcad to 79661c9 Compare October 20, 2016 14:59

LLVMAddTargetData was deprecated in 3.7 and removed in 3.9

Verified

This commit was signed with the committer’s verified signature.

headius Charles Oliver Nutter

GPG key ID: 8E26FF248BC7AEAD

Verified
Learn about vigilant mode

398441a

ysbaddaden force-pushed the fix-llvm-3.9 branch from 79661c9 to 67024f3 Compare October 20, 2016 15:33

ysbaddaden force-pushed the fix-llvm-3.9 branch from 67024f3 to 472530f Compare October 20, 2016 15:37

ysbaddaden added 3 commits October 20, 2016 17:40

LLVM_VERSION macros for LLVMExt (prepare for LLVM 4.0)

b9bc24d

Fix: --debug with LLVM 3.9 (inlinable function missing !dbg)

e573772

Some syntax sugar expansions didn't set the location for some generated `Crystal::Call` nodes, which is rejected by LLVM 3.9 when the parent definition has a `!dbg` location itself.

ysbaddaden force-pushed the fix-llvm-3.9 branch from 472530f to e573772 Compare October 20, 2016 15:40

ysbaddaden merged commit 13b11d7 into crystal-lang:master Oct 21, 2016

ysbaddaden deleted the fix-llvm-3.9 branch October 21, 2016 10:07

asterite pushed a commit that referenced this pull request Oct 21, 2016

Compiler specs: dispose LLVM JIT as soon as possible.

7517056

This seems to fix an issue with unwind stopping to work well after using a JIT without disposing it. See #3439

asterite pushed a commit that referenced this pull request Oct 21, 2016

Add a few more missing node locations. Related to #3439

4343d16

ysbaddaden added this to the 0.20.0 milestone Oct 25, 2016

straight-shoota mentioned this pull request Jan 14, 2024

osx flag #14238

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support LLVM 3.9 #3439

Support LLVM 3.9 #3439

ysbaddaden commented Oct 19, 2016 •

edited

Loading

Sija Oct 19, 2016 •

edited

Loading

ysbaddaden Oct 19, 2016

bcardiff commented Oct 19, 2016

ysbaddaden commented Oct 19, 2016

ysbaddaden commented Oct 19, 2016

asterite commented Oct 19, 2016

ysbaddaden commented Oct 19, 2016 •

edited

Loading

ysbaddaden commented Oct 19, 2016 •

edited

Loading

ysbaddaden commented Oct 20, 2016 •

edited

Loading

ilovezfs commented Oct 21, 2016

asterite commented Oct 21, 2016

ilovezfs commented Oct 21, 2016

ysbaddaden commented Oct 21, 2016

ilovezfs commented Oct 21, 2016

asterite commented Oct 21, 2016

asterite commented Oct 21, 2016

ilovezfs commented Oct 21, 2016 •

edited

Loading

asterite commented Oct 21, 2016

ysbaddaden commented Oct 21, 2016

asterite commented Oct 21, 2016

asterite commented Oct 21, 2016 •

edited

Loading

asterite commented Oct 21, 2016

ysbaddaden commented Oct 21, 2016

will commented Oct 22, 2016

ysbaddaden commented Oct 22, 2016

Support LLVM 3.9 #3439

Support LLVM 3.9 #3439

Conversation

ysbaddaden commented Oct 19, 2016 • edited Loading

Sija Oct 19, 2016 • edited Loading

Choose a reason for hiding this comment

ysbaddaden Oct 19, 2016

Choose a reason for hiding this comment

bcardiff commented Oct 19, 2016

ysbaddaden commented Oct 19, 2016

ysbaddaden commented Oct 19, 2016

asterite commented Oct 19, 2016

ysbaddaden commented Oct 19, 2016 • edited Loading

ysbaddaden commented Oct 19, 2016 • edited Loading

ysbaddaden commented Oct 20, 2016 • edited Loading

ilovezfs commented Oct 21, 2016

asterite commented Oct 21, 2016

ilovezfs commented Oct 21, 2016

ysbaddaden commented Oct 21, 2016

ilovezfs commented Oct 21, 2016

asterite commented Oct 21, 2016

asterite commented Oct 21, 2016

ilovezfs commented Oct 21, 2016 • edited Loading

asterite commented Oct 21, 2016

ysbaddaden commented Oct 21, 2016

asterite commented Oct 21, 2016

asterite commented Oct 21, 2016 • edited Loading

asterite commented Oct 21, 2016

ysbaddaden commented Oct 21, 2016

will commented Oct 22, 2016

ysbaddaden commented Oct 22, 2016

ysbaddaden commented Oct 19, 2016 •

edited

Loading

Sija Oct 19, 2016 •

edited

Loading

ysbaddaden commented Oct 19, 2016 •

edited

Loading

ysbaddaden commented Oct 19, 2016 •

edited

Loading

ysbaddaden commented Oct 20, 2016 •

edited

Loading

ilovezfs commented Oct 21, 2016 •

edited

Loading

asterite commented Oct 21, 2016 •

edited

Loading