-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Add riscv64imc-unknown-none-elf target #58012
Conversation
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
|
||
pub fn target() -> TargetResult { | ||
Ok(Target { | ||
data_layout: "e-m:e-i64:64-n64-S128".to_string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the correct data layout to pass to LLVM? I read https://llvm.org/docs/LangRef.html#data-layout but I'm not 100% sure I got everything correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. That version seems to be equivalent, since p:64:64:64
and f128:128:128
are default. Though I don't quite understand why the defaults have an extra section to them... Should I copy over the configuration from lowRISC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently rustc verifies that the data layout passed to LLVM matches the default data-layout for the target. IIRC it is an ICE or an error otherwise, don’t remember which exactly.
So you have to have exactly the same string as the backend you’re hoping to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it doesn't matter, but I think being consistent with the clang driver isn't a bad policy
Can you make the target |
+1 for riscv64-imacfd-unknown-none-elf (however, in riscv-spec-v2.2 it's spelled as imafdc, see table 22.1) |
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Not sure, but probably |
r? @nagisa |
Doesn't this need an update of the LLVM submodule as well to work?
There is no F & D support in LLVM yet. A G target can be added later, although I think it makes more sense to add OS-specific G targets. |
Pure riscv64ic code can run on a riscv64gc processor. Thus, we can claim the target supports F & D even if we never generate code that uses those extensions. When LLVM is suitably updated, anyone using the target will automatically get faster floating point support. I also think that 64bit G + C makes sense as a target because it corresponds to what any laptop, desktop or server will have. The other 64-bit variants are likely to only be useful on very specific embedded platforms that for some reason need to work with lots of memory but can't afford large cores. |
Travis CI now passes, but I'm not actually sure how to test these changes. Does Travis do any checking of new targets? Are there any tests outside of continuous integration that I should be running? |
Yes, you should build a compiler with your changes and
See https://rust-lang.github.io/rustc-guide/how-to-build-and-run.html |
That page indicates that tier 3 targets don't have official builds, but the other riscv targets do have toolchains distributed via rustup. Am I misunderstanding, or is there something else required to get included in rustup? |
Sorry, this was my mistake, see my updated comment. |
FU540-C000 uses RV64IMAC for their Monitor Core, so target riscv64imac-unknown-none-elf makes sense. If we have no F & D support, maybe we can implement support for IMAC target, and add support for GC target later. If it's ok to have GC target now, I can add IMAC target in a separate PR. |
I don't think we should have a target where using floating point types doesn't work only when the compiler gets to codegen. |
linker: Some("rust-lld".to_string()), | ||
cpu: "generic-rv64".to_string(), | ||
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86005 | ||
max_atomic_width: None, //Some(32), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we have atomics in nightly:
https://github.com/rust-lang/rust/blob/master/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs#L20-L21
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For rv32 yes, the rv64 target still is missing it I think. You can test it by trying to build this:
#![allow(warnings)]
#![crate_type = "lib"]
#![no_std]
use core::sync::atomic::{AtomicUsize, Ordering};
#[no_mangle]
pub fn foo(x: &AtomicUsize, y: usize) -> usize {
x.swap(y, Ordering::SeqCst)
}
Can someone write a small floating point example and make sure it generates appropriate code? I'm not sure if enabling the +f and +d extensions will prevent rust/llvm from using compiler-rt or not. |
Also since I haven't really looked into how floating point support works there might be some compiler-builtins missing. |
@jethrogb I've tried the build command you suggested but keep getting back an error about being unable to create an LLVM TargetMachine. The same thing happens even if I target the riscv32imac-unknown-none-elf target which is known to work.
@dvc94ch Floating point math does seem to work on the existing riscv32imac target. For instance, this code (with a custom println macro) produces correct output, although takes a few seconds to run: let mut i = 1.0;
let mut sum = 0.0f32;
while i < 40_000_000.0 {
sum += 1.0 / i - 1.0 / (i + 2.0);
i += 4.0;
}
println!("pi = {}", 4.0 * sum); |
@fintelia Same here. Building Rust with make doesn't help either. |
@fintelia probably, we need to uncomment this line in config.toml... |
Were you able to get it to compile with that change? It doesn't seem to have any effect for me |
@fintelia Now I can build riscv32* targets, but builds for riscv64* targets are failed for different reasons: |
@nagisa or @jethrogb do either of you have any suggestions on how to debug this? |
Debugging a hang is probably best by getting a coredump and loading it up in the debugger (or just attaching the debugger directly). LLVM ERROR: attach a debugger and get a callstack at the time this assertion is triggered; SIGSEGV/SIGBUS... coredump or debugger and look at the callstack/instructions around the area. |
I attached GDB and was able to establish that the hang seems to be happening within the codegen code while compiling libcore. Specifically, the function llvm::SelectionDAG::Combine from /r/r/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends/librustc_codegen_llvm-llvm.so never returns. Is there anyone with experience with this part of the code that might be able to dig further? Build output:
Backtrace:
|
That is a bug in the backend. You may have some success by looking at what LLVM is doing by looking at the output of |
I was able to get some output from Unfortunately I don't have more time to commit to this issue right now so if @Disasm, @dvc94ch or anyone else wants to take over, please do |
No thanks. Did you apply the latest patches and verify it works or did you
just use what happened to be in rusts llvm fork? That would be useful for
submitting a bug report or find out what is missing
…On Sat, Feb 9, 2019, 18:33 Jonathan Behrens ***@***.*** wrote:
I was able to get some output from --Cllvm-args=print-before="simplifycfg"but
llc rejected processing it (something about an argument mismatch a couple
hundred lines in). I tried a couple different ways of getting normal
bytecode/IR output from LLVM but was unable to anything working. I also
wasn't able to run any builds without relying on the bootstrap.py wrapper,
blocking any chance at producing a more minimal example.
Unfortunately I don't have more time to commit to this issue right now so
if @Disasm <https://github.com/Disasm>, @dvc94ch
<https://github.com/dvc94ch> or anyone else wants to take over, please do
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#58012 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAtRr_8yVbIU3-U3odXosOTAUyZBhr_Oks5vLwZUgaJpZM4abBet>
.
|
@fintelia Thanks for the effort! I will try to experiment with updated LLVM soon. |
"RISC-V Unix-class systems implement the RV64GC ISA with supervisor mode and the Sv39 page-based virtual-memory scheme." All Linux distributions are targeting RV64GC (Fedora, Debian, openSUSE, etc.) There are still missing some pieces. TLS is under review. PIC is missing implementation. Linux hard float ABIs are also not implemented. Not sure how all that affects Rust directly. Alex Bradbury presented the current status at FODEM 2019: https://fosdem.org/2019/schedule/event/riscvllvmclang/ |
I managed to compile Rust for RV64IMAC target with some LLVM patches, however rust-lld was not able to link a test application due to incompatible relocations. The problem is triggered by this code:
Assembly code:
Another assembly code (for
and linker doesn't complain about it. A linker script that is used to build the test application maps RAM region into 0x80000000 address, which is invalid for Changing Any ideas? |
CC @asb |
@Disasm Awesome progress! I'll mention some stuff you can try for completeness, even though I'm sure you thought of it yourself:
|
@dvc94ch I'm able to link with different RAM base addresses (0x40000000 for example), even float instructions are generated for RV64GC target. The main problem is that Rust or LLVM generates wrong instructions (static loads instead of pc-relative) which renders Rust toolchain useless for any addresses >=0x80000000. Trying to figure out the cause of this problem... |
Looks like the answer is "not supported yet".
So... I suggest adding support for both |
True, the PIC is not yet supported in LLVM for RISC-V. Regarding |
@davidlt I personally do not need |
These changes are based on 8dfd5c3 which added the riscv32imc target.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Superseded by #58406 |
Add riscv64{imac,gc}-unknown-none-elf targets Previous attempt by @fintelia: #58012 Related: rust-embedded/wg#218
Add riscv64{imac,gc}-unknown-none-elf targets Previous attempt by @fintelia: #58012 Related: rust-embedded/wg#218
Add riscv64{imac,gc}-unknown-none-elf targets Previous attempt by @fintelia: #58012 Related: rust-embedded/wg#218
These changes are based on 8dfd5c3 which added the riscv32imc target.
CC: rust-embedded/wg/issues/218