Skip to content

Upgrade to support LLVM 4.0 (development) #22

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

Closed
dylanmckay opened this issue Oct 6, 2016 · 26 comments
Closed

Upgrade to support LLVM 4.0 (development) #22

dylanmckay opened this issue Oct 6, 2016 · 26 comments

Comments

@dylanmckay
Copy link
Member

dylanmckay commented Oct 6, 2016

I get this in the tip of avr-support-4.0

configure --enable-debug --disable-docs --enable-llvm-assertions --enable-debug-assertions --enable-rustbuild
make rustc-stage1
Building stage1 std artifacts (x86_64-apple-darwin -> x86_64-apple-darwin)
   Compiling core v0.0.0 (file:///Users/dylanmckay/projects/avr-rust/rust/src/libcore)

Assertion failed: (isa<X>(Val) && "cast<Ty>() argument of incompatible type!"), function cast, file /Users/dylanmckay/projects/builds/avr-rust/rust/build/x86_64-apple-darwin/llvm/include/llvm/Support/Casting.h, line 237.
error: Could not compile `core`.

This triggers it

 build/x86_64-apple-darwin/stage1/bin/rustc /Users/dylanmckay/projects/avr-rust/rust/src/libcore/lib.rs --crate-name core --crate-type lib -C debug-assertions=off -C metadata=877272a66b7a4e4d -C extra-filename=-877272a66b7a4e4d --out-dir ./libcore --target x86_64-apple-darwin -L dependency=/Users/dylanmckay/projects/builds/avr-rust/rust/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/debug/deps --cfg stage1 --sysroot /Users/dylanmckay/projects/builds/avr-rust/rust/build/x86_64-apple-darwin/stage1 -g -C debug-assertions=y -C codegen-units=1 --emit llvm-ir -o libcore.ll
@shepmaster
Copy link
Member

shepmaster commented Oct 8, 2016

(lldb) p *Val
(llvm::Metadata) $0 = (SubclassID = '\x90', Storage = '�', SubclassData16 = 4046, SubclassData32 = 1)

Coming from RustWrapper.cpp:

-> 319  DEFINE_ISA_CONVERSION_FUNCTIONS(Metadata, LLVMRustMetadataRef)

   327  DIT* unwrapDIptr(LLVMRustMetadataRef ref) {
-> 328      return (DIT*) (ref ? unwrap<MDNode>(ref) : NULL);
   329  }

   566  #if LLVM_VERSION_GT_OR_EQ(4,0)
-> 567          unwrapDI<DIExpression>((LLVMRustMetadataRef) Val),
   568  #else

From rustc_trans::debuginfo::metadata::create_global_var_metadata in metadata.rs:

-> 1939         llvm::LLVMRustDIBuilderCreateStaticVariable(DIB(cx),

@shepmaster
Copy link
Member

Hard coding that to nullptr gets to the next error:

Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior to being destroyed).

appears to be coming from:

   160  LLVMRustArchiveIteratorFree(LLVMRustArchiveIteratorRef rai) {
-> 161      delete rai;
   162  }

@shepmaster
Copy link
Member

temporarily removing the error check and I've gotten back to compiling stage2. That seems to indicate that the end of this set of upgrades is in sight!

@dylanmckay
Copy link
Member Author

This is way more work that I remember it being last time lol

@shepmaster
Copy link
Member

@dylanmckay heads up - I'm going to do a rebase on Rust master and squash some things together. I want to post something to upstreams issue tracker about upgrading to 4.0 because it seems so difficult. Do you have any outstanding work?

@nateozem
Copy link

nateozem commented Nov 6, 2016

shepmaster: "temporarily removing the error check and I've gotten back to compiling stage2"

I'm looking into this issue, but I'm not sure if anybody settled a solution for this.
Will the error check be removed or is there a way to satisfy the test?

@shepmaster shepmaster changed the title Assertion failed: (isa<X>(Val) && "cast<Ty>() argument of incompatible type!") Upgrade to support LLVM 4.0 (development) Nov 6, 2016
@shepmaster
Copy link
Member

@nateozem it's tricky 😈

There are two competing projects to keep up with: Rust and (AVR-)LLVM. Working against any given "older" version is potentially a waste of time; things might have changed between then and the present and there would be wasted work.

On my laptop (and not pushed anywhere yet), I am attempting to integrate avr-llvm/llvm@3fa7cf1 and rust-lang/rust@cae6ab1, which are the latest commits in each repository at this point in time. This also means that I am getting a new error, different from the others above. 😸

Generally, my approach has been "quickly hack up Rust to get it to compile, then go back and tease out each fix to prepare a proper PR". I'm still in the "quickly hack up" part, so saying whether any given hack will be preserved is difficult at best.

@shepmaster
Copy link
Member

I've also created rust-lang#37609 to potentially get suggestions and guidance from people more familiar with the marriage of Rust + LLVM, as well as a way to ease getting the changes upstreamed.

@dylanmckay
Copy link
Member Author

No outstanding work @shepmaster.

Your description on rust-lang#37609 sounds 💯

@shepmaster
Copy link
Member

Ok, I've rebased, cleaned up, and force-pushed to avr-support-4.0 (de6eefe581ae70452324110cea534a970639b9f0). I was able to completely build the compiler with this branch and an out-of-tree build of AVR-LLVM with Rust support (avr-llvm/llvm@d93e840). I have not attempted to actually compile any AVR code with it 😇

@dsvensson
Copy link

dsvensson commented Nov 18, 2016

Sorry to add noise here, but I'm super excited about the AVR support and just tried to build with ../configure --target=avr-atmel-none --disable-jemalloc, and in the Entering 'src/rust-installer I get an error that AVR target doesn't exist. Is that just reality, or is the README not updated to the latest state of things?

@shepmaster
Copy link
Member

he README not updated to the latest state of things?

Not updated, and the code isn't really ready yet. I'm working on it, I swear!

@dylanmckay
Copy link
Member Author

By the way @shepmaster, there is technically one difference between the downstream and upstream versions of the AVR backend.

In downstream, the backend is setup in CMake to be a first-class backend (enabled by default), but upstream it is set up as an "experimental" backend, which must be enabled by cmake -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=AVR.

We will need to add this flag when we add AVR support to Rust properly.

@dsvensson
Copy link

From a bystander perspective, the only checklist item in rust-lang#37609 not merged is "powidf2 / powisf2", and the LLVM change mentioned in the comments that's not in the checklist. Does this mean the world is near having rustc generating AVR binaries, or is there more work left that hasn't been mentioned yet? Anywhere near a timeframe guess? (like days, weeks, months)

@shepmaster
Copy link
Member

@dylanmckay So, since Rust is getting close to supporting LLVM 4.0, I decided to give compiling over here another shot. Strangely, it looks like LLVM is half-building the AVR support:

Undefined symbols for architecture x86_64:
  "llvm::AVRMCRegisterClasses", referenced from:
      llvm::AVRRegisterInfo::AVRRegisterInfo() in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      llvm::AVR::GPR8RegClass in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      llvm::AVR::GPR8loRegClass in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      llvm::AVR::LD8RegClass in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      llvm::AVR::LD8loRegClass in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      llvm::AVR::CCRRegClass in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      llvm::AVR::DREGSRegClass in libLLVMAVRCodeGen.a(AVRRegisterInfo.cpp.o)
      ...

You can see my horrible hacks in progress at https://github.com/avr-rust/rust/tree/avr-support-4.0-wip

@dylanmckay
Copy link
Member Author

Very weird.

It looks like you haven't pushed some changes - when I open your src/llvm submodule it gives me a 404.

@shepmaster
Copy link
Member

@dylanmckay ah yes, I don't deal with submodules frequently so I tend to forget them. I've just pulled in the emscripten incoming branch, without any Rust-specific additions.

@shepmaster
Copy link
Member

(pushed the submodule change now)

@shepmaster
Copy link
Member

Here's what I see:

lib/Target/AVR/AVRGenRegisterInfo.inc defines AVRMCRegisterClasses, but only if GET_REGINFO_MC_DESC is defined when it is #included. I cannot see anywhere that AVR code defines this. All the other architectures seem to define it, in lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp, for example. However, that file does not exist in the emscripten fork:

$ less lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.cpp
lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.cpp: No such file or directory

Perhaps there was a mishap when LLVM 4.0 got merged into there? Maybe I've got the wrong branch?

@shepmaster
Copy link
Member

Oh, maybe I should be using next-merge instead of incoming...

@shepmaster
Copy link
Member

Yep, that was it. I've updated the avr-support-4.0 branch with my code. Any reason to keep the old avr-support branch?

@shepmaster
Copy link
Member

Well, maybe I should try to compile something first ;-)

@dylanmckay
Copy link
Member Author

Oh, maybe I should be using next-merge instead of incoming...

lol

Definitely replace avr-support with avr-support-4.0. I'm not personally really bothered because there's not much point using this fork as the 4.0 upgrade is so close.

@shepmaster
Copy link
Member

shepmaster commented Feb 14, 2017

there's not much point using this fork as the 4.0 upgrade is so close.

Perhaps I'm missing something, but this fork still contains all of the AVR-specific changes to Rust, which is distinct from the LLVM 4 changes. Do you foresee those being merged into Rust proper in the near future?

@dylanmckay
Copy link
Member Author

dylanmckay commented Feb 15, 2017

Historically the 16-bit specific patches were pretty welcome into Rust proper.

On top of this, my impression of the MSP430 backend:

  • Someone realised there was a backend we could use
  • Someone wrote a PR
  • Everyone just went ¯_(ツ)_/¯ why not

I feel like if the MSP430 target could get used, surely the AVR backend (with a much bigger userbase to my knowledge) would also get merged.

Also, in my experience, updating this repo is normally easier to just start from Rust master, grep for Mips, copy it and add the AVR target stuff in again. The last time I did update this was when the "JSON target specification" stuff landed though.

@shepmaster
Copy link
Member

Wew. Done in avr-support as 1db0bbf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants