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

LLVM upgrade #5368

Closed
sanxiyn opened this issue Mar 14, 2013 · 13 comments
Closed

LLVM upgrade #5368

sanxiyn opened this issue Mar 14, 2013 · 13 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one.
Milestone

Comments

@sanxiyn
Copy link
Member

sanxiyn commented Mar 14, 2013

Let's upgrade LLVM to the latest trunk.

Since LLVM upgrades are currently disruptive to the infrastructure, all of the following should be done together:

@brson
Copy link
Contributor

brson commented Mar 15, 2013

Oops, I didn't see this and opened #5401. I'll reproduce my comments here.

@brson
Copy link
Contributor

brson commented Mar 15, 2013

Probably should also incorporate #3775 once we rebase to trunk.

Consider #5403 too.

My wip llvm removes the GC patches, removes clang and adds your ARM patches. It doesn't rebase onto LLVM trunk, though, nor resolve #3775, which will probably be solved by resetting compiler-rt to trunk and dropping our patches there.

I'm going to test this here today, but I'll be out of the office most of next week so won't be able to land it yet. @ILyoan if you want to speed this along you might want to try rebasing 'wip' onto LLVM/compiler-rt trunk and try to get it building on linux/mac/windows. Since you have commit access you can test against the bots by pushing to mozilla's try branch. Then you can go to the buildbot's builders page and force builds of all the try branches. Note that to rebuild LLVM on the bots you need to set 'Property 1 Name' to clean-llvm and the value to 1. Also 'revision' needs to be the commit hash and (I think) 'branch' needs to be 'try'.

@sanxiyn
Copy link
Member Author

sanxiyn commented Mar 16, 2013

There were two issues while upgrading to trunk:

@ILyoan
Copy link
Contributor

ILyoan commented Mar 16, 2013

@brson Thanks for your help!!
We are preparing to send ARM morestack patch to upstream. I think we can send the path next week.
I'm expecting that there will be a couple of modifications to get ARM unwinding to work but I'm not sure if it is essential thing to be merged before 0.6.(At least I think it should be merged in 0.7 though)

@yichoi
Copy link
Contributor

yichoi commented Mar 16, 2013

@brson depends on the progress of ARM unwinding. our priority is to enable debug information on ARM stack frame. I think ARM unwinding and ARM stack frame are same problem.
@ILyoan proceed ARM stack frame first, I will manage to handle rebase brson's tree with LLVM trunk

This was referenced Mar 16, 2013
@yichoi
Copy link
Contributor

yichoi commented Mar 19, 2013

@brson I am investigating your wip branch. Is compiler-rt necessary ? I am going to fork https://github.com/llvm-mirror/llvm not https://github.com/chapuni/llvm.

@brson
Copy link
Contributor

brson commented Mar 19, 2013

I believe it is, though I don't know why.
On Mar 19, 2013 5:02 AM, "yichoi" notifications@github.com wrote:

@brson https://github.com/brson I am investigating your wiphttps://github.com/brson/llvm/tree/wipbranch. Is compiler-rt necessary ? I am going to fork
https://github.com/llvm-mirror/llvm not https://github.com/chapuni/llvm.


Reply to this email directly or view it on GitHubhttps://github.com//issues/5368#issuecomment-15110439
.

bors added a commit that referenced this issue Mar 20, 2013
Partial Fix for #5265

- Enabling LLVM ARM ehabi option.
- Add ARM debug information manually for ccall.s
- Compile object file using Android-NDK.

Current LLVM trunk version can generate ARM debug information for assembly files but it is incomplete for object files. Unwinding on ARM can be done with LLVM trunk(the LLVM submodule of rust has problem on generating ARM debug information). See #5368

The Android-NDK detour(0f89eab) can be removed after LLVM has complete feature of generating ARM debug information for object file.
@yichoi
Copy link
Contributor

yichoi commented Mar 20, 2013

@brson wip2 forked from Latest LLVM plus our ARM patch works without clang and compiler-rt. Latest Rust with LLVM wip2 maintained Here. All test passed on linux except debug-info. I think debugging format could be changed.

@sanxiyn sanxiyn mentioned this issue Mar 26, 2013
4 tasks
@brson
Copy link
Contributor

brson commented Mar 28, 2013

I tried several different variations of branchs that pulled together all the necessary changes and ended up with this one here. It includes the __morestack changes to rust and the fix for the OS X metadata changes in LLVM. The only problem I'm seeing is that when building on x86_64-unknown-linux-gnu with --disable-optimize the stage1 build of librustc segfaults. I looked into it for a few hours but didn't solve anything.

@yichoi
Copy link
Contributor

yichoi commented Mar 28, 2013

I was wrong. I misunderstood disable optimization as --disable-optimize-llvm then tried which was successful but --disable-optimize failed like you.
I tried with LLVM rebased with latest one. but also failed.

@yichoi
Copy link
Contributor

yichoi commented Mar 28, 2013

If one split the compile to --emit-llvm and running llc on LLVM IR, no segfault happens. It looks to me that something is missed in the initialization

@brson
Copy link
Contributor

brson commented Mar 28, 2013

I agree something isn't initialized correctly.

@brson
Copy link
Contributor

brson commented Mar 28, 2013

Adding Target->addAnalysisPasses(*PM); to RustWriteOutputFile fixes it.

@sanxiyn sanxiyn closed this as completed Apr 11, 2013
bors added a commit to rust-lang-ci/rust that referenced this issue May 2, 2020
…n, r=matthiaskrgr

Move verbose_file_reads to restriction

cc rust-lang#5368

Using `File::read` instead of  `fs::read_to_end` does make sense in multiple cases, so this lint is rather restriction, than complexity

changelog: Move [`verbose_file_reads`] to restriction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

4 participants