Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to build against llvm 3.6 rc1 #21512

Closed
sylvestre opened this Issue Jan 22, 2015 · 15 comments

Comments

Projects
None yet
6 participants
@sylvestre
Copy link
Contributor

sylvestre commented Jan 22, 2015

With the following error:

g++  -O2  -Wall -Werror -g -fPIC -m64 -fno-rtti -MMD -MP -MT  x86_64-unknown-linux-gnu/rustllvm/ExecutionEngineWrapper.o -MF x86_64-unknown-linux-gnu/rustllvm/ExecutionEngineWrapper.d -c -o  x86_64-unknown-linux-gnu/rustllvm/ExecutionEngineWrapper.o    -iquote /usr/lib/llvm-3.6/include  -DNDEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -g -O2 -fomit-frame-pointer -std=c++11 -fvisibility-inlines-hidden -fno-exceptions -fPIC -ffunction-sections -fdata-sections -Wcast-qual  -iquote /usr/lib/llvm-3.6/include -iquote /home/sylvestre/dev/debian/rust/src/rustllvm/include /home/sylvestre/dev/debian/rust/src/rustllvm/ExecutionEngineWrapper.cpp
/home/sylvestre/dev/debian/rust/src/rustllvm/ExecutionEngineWrapper.cpp: In function ‘LLVMOpaqueExecutionEngine* LLVMBuildExecutionEngine(LLVMModuleRef, LLVMRustJITMemoryManagerRef)’:
/home/sylvestre/dev/debian/rust/src/rustllvm/ExecutionEngineWrapper.cpp:97:34: error: no matching function for call to ‘llvm::EngineBuilder::setMCJITMemoryManager(RustJITMemoryManager*&)’
         .setMCJITMemoryManager(mm)
                                  ^
/home/sylvestre/dev/debian/rust/src/rustllvm/ExecutionEngineWrapper.cpp:97:34: note: candidate is:
In file included from /home/sylvestre/dev/debian/rust/src/rustllvm/rustllvm.h:34:0,
                 from /home/sylvestre/dev/debian/rust/src/rustllvm/ExecutionEngineWrapper.cpp:11:
/usr/lib/llvm-3.6/include/llvm/ExecutionEngine/ExecutionEngine.h:528:18: note: llvm::EngineBuilder& llvm::EngineBuilder::setMCJITMemoryManager(std::unique_ptr<llvm::RTDyldMemoryManager>)
   EngineBuilder &setMCJITMemoryManager(std::unique_ptr<RTDyldMemoryManager> mcjmm);
                  ^
/usr/lib/llvm-3.6/include/llvm/ExecutionEngine/ExecutionEngine.h:528:18: note:   no known conversion for argument 1 from ‘RustJITMemoryManager*’ to ‘std::unique_ptr<llvm::RTDyldMemoryManager>’
/home/sylvestre/dev/debian/rust/mk/rustllvm.mk:47: recipe for target 'x86_64-unknown-linux-gnu/rustllvm/ExecutionEngineWrapper.o' failed
make[2]: *** [x86_64-unknown-linux-gnu/rustllvm/ExecutionEngineWrapper.o] Error 1
@lucab

This comment has been minimized.

Copy link
Contributor

lucab commented Jan 27, 2015

The embedded fork is stuck somewhere between 3.5 and 3.6. I managed to rebase it without too many issues at https://github.com/lucab/llvm/tree/lucab/rust-llvm-3.6.0rc1.

On the other hand, rustllvm needs some facelifting, as there have been at least the following breaking changes in the middle:

  • split between Metadata and Value, rev223802
  • new intrinsic syntax for llvm.dbg.declare/llvm.dbg.value as described here, rev218787
@lucab

This comment has been minimized.

Copy link
Contributor

lucab commented Jan 27, 2015

On the bright side, our current delta to 3.6.0rc1 is:

  • a NullCheckElimination pass. I spoke with @zwarich who told me this is too specific to rust (and the upstream status of a range analysis pass not too clear), so will stay there for now
  • a small optimization to getPassInfo(). The story of this is a bit unfortunate, and got no reaction when sent to upstream ML.
@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

FYI: I've been looking into the metadata/value split issue since that mostly affects debuginfo. Also, the llvm.dbg.declare/llvm.dbg.value changes require a bit more work if we also still want to be able to compile against LLVM < 3.6. I'm currently working on getting that fixed.

@richo

This comment has been minimized.

Copy link
Contributor

richo commented Jan 29, 2015

I started poking at this, and then successfully nerdsniped Alex into having another look. Our (mostly his) work so far is here if you want a starting point: https://github.com/alexcrichton/rust/tree/update-llvm

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

Let's pull @alexcrichton into the discussion.

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

This looks very similar to what I've done, naturally. But just following the metadata split will make it impossible to support both 3.5 and 3.6. I plan to solve this problem by providing a more abstract API for declaring local variables in debuginfo and implement a bit more logic RustWrapper.cpp, where we can better special-case on LLVM version.

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

For reference, here is the branch I'm working on:
https://github.com/michaelwoerister/rust/tree/llvm-36-preparations

It only concerns itself with the metadata/debuginfo changes.

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

I plan to solve this problem by providing a more abstract API for declaring local variables in debuginfo

A different approach would be to add a version suffix to the wrapped API functions, as in

LLVMDIBuilderInsertDeclare_3_5(...)
LLVMDIBuilderInsertDeclare_3_6(...)

That way we would not have to put logic into the wrapper.

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

Hm, I'm getting strange runtime errors from code compiled with rustc built against LLVM 3.5.0 and 3.5.1:

error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: http://doc.rust-lang.org/complement-bugreport.html
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'assertion failed: self.raw.hash != self.hashes_end', /home/mw/rust/src/libstd/collections/hash/table.rs:772
@sylvestre

This comment has been minimized.

Copy link
Contributor Author

sylvestre commented Jan 29, 2015

PR #21588 fixes this issue.

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

michaelwoerister commented Jan 29, 2015

@sylvestre Thanks!
It seems though that this particular issue is still open: #21620

I will now focus on making everything compile with LLVM 3.5 and 3.6. Then everything should just work if the above issue gets fixed in a 3.5.2 release.

@lucab

This comment has been minimized.

Copy link
Contributor

lucab commented Jan 29, 2015

@michaelwoerister Feel free to cherry-pick the commits from that PR in your queue. It was not merged at that point as 3.5.1 won't work anyway. However, if we manage to get the fixes into a 3.5.2 and you are touching rustllvm anyway, I think it makes sense to include them and hope for the best.

@richo

This comment has been minimized.

Copy link
Contributor

richo commented Jan 29, 2015

Is there context for why we want to support llvm 3.5 and 3.6?

Not saying we shouldn't, but I am curious why it's an explicit goal.

On Thu, Jan 29, 2015 at 2:28 PM, Luca Bruno notifications@github.com
wrote:

@michaelwoerister https://github.com/michaelwoerister Feel free to
cherry-pick the commits from that PR in your queue. It was not merged at
that point as 3.5.1 won't work anyway. However, if we manage to get the
fixes into a 3.5.2 and you are touching rustllvm anyway, I think it makes
sense to include them and hope for the best.


Reply to this email directly or view it on GitHub
#21512 (comment).

@lucab

This comment has been minimized.

Copy link
Contributor

lucab commented Jan 29, 2015

@richo discussion at http://internals.rust-lang.org/t/targeted-llvm-for-1-0/1371

Maybe not an explicit goal, but it looks like there was general interest. That made me try, and I've document in the related bugs all the shortcomings.

@dotdash

This comment has been minimized.

Copy link
Contributor

dotdash commented Feb 28, 2015

We're building against 3.6 by now.

@dotdash dotdash closed this Feb 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.