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.

Sign up for GitHub

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

Jump to bottom

Add support for pc-relative addressing on 64-bit RISC-V #62281

Merged
merged 2 commits into from
Jul 7, 2019
Merged

Add support for pc-relative addressing on 64-bit RISC-V #62281

merged 2 commits into from
Jul 7, 2019

Conversation

Disasm
Copy link
Contributor

@Disasm Disasm commented Jul 1, 2019

These changes allow Rust to generate position-independent code on riscv64 targets with code model medium.

Closes: #59802
See also: rust-embedded/riscv-rt#25, rust-embedded/wg#218

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2019
@Disasm
Copy link
Contributor Author

Disasm commented Jul 1, 2019

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned cramertj Jul 1, 2019
@nagisa
Copy link
Member

nagisa commented Jul 1, 2019

Is it necessary to change the default code_model for this to work (as opposed to "just" upgrading LLVM)?

@Disasm
Copy link
Contributor Author

Disasm commented Jul 1, 2019

@nagisa Yes, without medium code model LLVM uses absolute addresses.
https://godbolt.org/z/9y-S06 - default code model
https://godbolt.org/z/OnORw9 - "medium" code model

@nagisa
Copy link
Member

nagisa commented Jul 1, 2019 •
edited
Loading

@bors r+

Yes, without medium code model LLVM uses absolute addresses.

I was rhetorically asking whether just adding support requires changing defaults. In most cases, including this one, you don’t need to change defaults because people can switch to those behaviours by setting a flag or two (-Ccode-model=medium in this case). Approving this PR because relative addressing by default is probably a good idea either way.

@bors
Copy link
Contributor

bors commented Jul 1, 2019

📌 Commit c65ffa7 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2019
@bors
Copy link
Contributor

bors commented Jul 1, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jul 1, 2019

📌 Commit c65ffa7 has been approved by nagisa

@Disasm
Copy link
Contributor Author

Disasm commented Jul 1, 2019

Hmm, I didn't know about this variant, thanks for pointing out!

Centril added a commit to Centril/rust that referenced this pull request Jul 6, 2019
@Centril
Rollup merge of rust-lang#62281 - Disasm:riscv-pic, r=nagisa
5c6b58e 
Add support for pc-relative addressing on 64-bit RISC-V

These changes allow Rust to generate position-independent code on `riscv64` targets with code model `medium`.

Closes: rust-lang#59802
See also: rust-embedded/riscv-rt#25, rust-embedded/wg#218
@Centril Centril mentioned this pull request Jul 6, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 6, 2019
@Centril
Rollup merge of rust-lang#62443 - Centril:rollup-dksavfb, r=Centril
f499ffa 
Rollup of 4 pull requests [2]

Successful merges:

 - rust-lang#59800 (rustc: Remove `dylib` crate type from most rustc crates)
 - rust-lang#60081 (Refactor unicode.py script)
 - rust-lang#62270 (Move async-await tests from run-pass to ui)
 - rust-lang#62281 (Add support for pc-relative addressing on 64-bit RISC-V)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jul 7, 2019

⌛ Testing commit c65ffa7 with merge 4393768...

bors added a commit that referenced this pull request Jul 7, 2019
@bors
Auto merge of #62281 - Disasm:riscv-pic, r=nagisa
4393768 
Add support for pc-relative addressing on 64-bit RISC-V

These changes allow Rust to generate position-independent code on `riscv64` targets with code model `medium`.

Closes: #59802
See also: rust-embedded/riscv-rt#25, rust-embedded/wg#218
@bors
Copy link
Contributor

bors commented Jul 7, 2019

☀️ Test successful - checks-azure, checks-travis, status-appveyor
Approved by: nagisa
Pushing 4393768 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 7, 2019
@bors bors merged commit c65ffa7 into rust-lang:master Jul 7, 2019
@Disasm Disasm deleted the riscv-pic branch July 7, 2019 13:13
This was referenced Jul 9, 2019
Closed
Closed
laanwj added a commit to laanwj/k210-pac that referenced this pull request Jul 9, 2019
@laanwj
Remove work-around for pcrel issue
2f441ce 
With rust-lang/rust#62281, rust compiles in mcmodel=medium mode
by default and this work-around is no longer needed.
@laanwj laanwj mentioned this pull request Jul 9, 2019
Merged
@SimonSapin SimonSapin mentioned this pull request Jul 16, 2019
Closed
@rjoleary rjoleary mentioned this pull request Aug 25, 2019
Open
laanwj added a commit to laanwj/k210-pac that referenced this pull request Aug 29, 2019
@laanwj
Remove work-around for pcrel issue
5d3b7ee 
With rust-lang/rust#62281, rust compiles in mcmodel=medium mode
by default and this work-around is no longer needed.
@laanwj laanwj mentioned this pull request Jan 19, 2020
Merged
alexcrichton pushed a commit to rust-lang/cc-rs that referenced this pull request Jan 21, 2020
@laanwj @alexcrichton
risc-v: Use medany model (#467)
9228de6 
With rust-lang/rust#62281, rust compiles in mcmodel=medium mode
which is equivalent to GCC medany. This prevents linker relocation
errors code is placed outside the range `-0x80000000..0x7ffffffff`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nagisa nagisa

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RISC-V -mcmodel=medium
5 participants
@Disasm @rust-highfive @nagisa @bors @cramertj