[RISCV] Use lld as the default linker; Enable C extension; Add riscv32imc-unknown-none-elf target #53822

dvc94ch · 2018-08-30T12:31:01Z

The riscv32imc-unknown-none-elf target is intended for soft cores.

The riscv32imc target is supported by the following popular soft cores:

picorv32: https://github.com/cliffordwolf/picorv32
vexriscv: https://github.com/SpinalHDL/VexRiscv
pulp riscy: https://github.com/pulp-platform/riscv
pulp zero-riscy: https://github.com/pulp-platform/zero-riscy

rust-highfive · 2018-08-30T12:31:04Z

r? @varkor

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

japaric · 2018-08-30T12:40:49Z

r? @japaric

japaric

Just to confirm these IMC targets do not even support atomic loads and stores? Or do they support atomic loads and stores but not CAS operations?

japaric · 2018-08-30T12:39:27Z

src/librustc_target/spec/riscv32imc_unknown_none_elf.rs

+        target_env: String::new(),
+        target_vendor: "unknown".to_string(),
+        arch: "riscv32".to_string(),
+        linker_flavor: LinkerFlavor::Ld,


Shouldn't this be LinkerFlavor::Lld(LldFlavor::Ld)?

japaric · 2018-08-30T12:42:07Z

src/librustc_target/spec/riscv32imc_unknown_none_elf.rs

+            panic_strategy: PanicStrategy::Abort,
+            relocation_model: "static".to_string(),
+            emit_debug_gdb_scripts: false,
+            abi_blacklist: vec![


if this is the same list as the one used in the IMAC target could you refactor it into a function? See arm_base::abi_blacklist.

japaric · 2018-08-30T12:43:30Z

src/librustc_target/spec/riscv32imac_unknown_none_elf.rs

@@ -25,11 +25,11 @@ pub fn target() -> TargetResult {
        linker_flavor: LinkerFlavor::Ld,


Shouldn't this be LinkerFlavor::Lld(LldFlavor::Ld)?

japaric · 2018-08-30T12:43:45Z

src/librustc_target/spec/riscv32imac_unknown_none_elf.rs

            cpu: "generic-rv32".to_string(),
            max_atomic_width: Some(32),
            atomic_cas: false, // incomplete +a extension
-            features: "+m,+a".to_string(), // disable +c extension
+            features: "+m,+a,+c".to_string(),


japaric · 2018-08-30T12:45:54Z

src/librustc_target/spec/riscv32imac_unknown_none_elf.rs

@@ -25,11 +25,11 @@ pub fn target() -> TargetResult {
        linker_flavor: LinkerFlavor::Ld,

        options: TargetOptions {
-            linker: Some("riscv32-unknown-elf-ld".to_string()),
+            linker: Some("rust-lld".to_string()),
            cpu: "generic-rv32".to_string(),
            max_atomic_width: Some(32),
            atomic_cas: false, // incomplete +a extension


Just checking: this hasn't been fixed yet?

Nope, a few commits have happened but nothing that "interesting".

dvc94ch · 2018-08-30T14:07:10Z

Just to confirm these IMC targets do not even support atomic loads and stores? Or do they support atomic loads and stores but not CAS operations?

Ah yes, loads and stores up to pointer width should be atomic.

japaric · 2018-08-30T17:27:31Z

Ah yes, loads and stores up to pointer width should be atomic.

LLVM says otherwise:

#![no_std]

use core::sync::atomic::{Ordering, AtomicUsize};

#[no_mangle]
pub fn foo(x: &AtomicUsize) -> usize {
    x.load(Ordering::SeqCst)
}

IMAC

$ $(rustc +nightly --print sysroot)/bin/llvm-objdump -d target/riscv32imac-unknown-none-elf/release/libfoo.rlib

target/riscv32imac-unknown-none-elf/release/libfoo.rlib(foo-5c3d4a3cd43899d6.foo.axz7xdhy-cgu.0.rcgu.o): file format ELF32-riscv

Disassembly of section .text.foo:
foo:
       0:       0f 00 30 03     fence   rw, rw
       4:       08 41   lw      a0, 0(a0)
       6:       0f 00 30 02     fence   r, rw
       a:       82 80   ret

$ $(rustc +nightly --print sysroot)/bin/llvm-nm target/riscv32imac-unknown-none-elf/release/libfoo.rlib

foo-5c3d4a3cd43899d6.foo.axz7xdhy-cgu.0.rcgu.o:
00000000 T foo

IMC

$ $(rustc +nightly --print sysroot)/bin/llvm-objdump -d target/riscv32imc-unknown-none-elf/release/libfoo.rlib
target/riscv32imc-unknown-none-elf/release/libfoo.rlib(foo-5c3d4a3cd43899d6.foo.axz7xdhy-cgu.0.rcgu.o): file format ELF32-riscv

Disassembly of section .text.foo:
foo:
       0:       41 11   addi    sp, sp, -16
       2:       06 c6   sw      ra, 12(sp)
       4:       95 45   addi    a1, zero, 5
       6:       97 00 00 00     auipc   ra, 0
       a:       e7 80 00 00     jalr    ra
       e:       b2 40   lw      ra, 12(sp)
      10:       41 01   addi    sp, sp, 16
      12:       82 80   ret

$(rustc +nightly --print sysroot)/bin/llvm-nm target/riscv32imc-unknown-none-elf/release/libfoo.rlib

foo-5c3d4a3cd43899d6.foo.axz7xdhy-cgu.0.rcgu.o:
         U __atomic_load_4
00000000 T foo

So the IMC target should have max_atomic_width: None.

P.S. Could you push new commits rather than rewriting the existing ones? New commits are easier to review.

dvc94ch · 2018-08-30T17:46:07Z

Mmh fence is part of the base ISA so I don't know why the above code wouldn't work on imc in this particular case.

hanna-kruppe · 2018-08-30T17:51:51Z

AFAIK only FENCE.I is in the base ISA, and that one is of no use for implementing atomics.

dvc94ch · 2018-08-30T17:59:58Z

@rkruppe It's in the user spec 2.2 section 2.7 memory model as part of the base ISA.

dvc94ch · 2018-08-30T18:17:06Z

It is my non-expert opinion that llvm is wrong in this case, so we should leave it as is. Since this target is intended for running on fpga's, they are usually single core anyway...

hanna-kruppe · 2018-08-30T18:27:28Z

Oh, right, this is due to the problem described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86005 -- tl;dr can't emit fence-based inline sequences for some operations and libcalls for other operations on the same memory location because those two might not synchronize correctly with each other.

japaric · 2018-08-30T18:28:41Z

@dvc94ch could you file a bug report in LLVM's bugzilla?

Rust targets must not produce binaries with undefined references (i.e. that fail to link) so the IMC target must have max_atomic_width: None until the bug is fixed then the workaround can be removed.

hanna-kruppe · 2018-08-30T18:38:46Z

If depending on __atomic* functions is not acceptable, I don't think we can ever set max_atomic_width to anything other than None for non-A targets, because max_atomic_width=N implies the availability of CAS and read-modify-write operations that are infeasible to implement (on a multicore system) without the A extension.

dvc94ch · 2018-08-30T18:42:27Z

Then what is the purpose of 173c679#diff-b74c18bf827643ac908307f62d9c1d60R32?

japaric · 2018-08-30T18:44:00Z

@dvc94ch thanks!

@bors r+

@rkruppe max_atomic_width: N and atomic_cas: false make the Atomic*.{load,store} API available and leave out the rest of the API.

bors · 2018-08-30T18:44:01Z

📌 Commit 173c679 has been approved by japaric

hanna-kruppe · 2018-08-30T18:47:52Z

Oh, good to know! However, as mentioned above there are very good reasons for LLVM to expand atomic loads and stores to libcalls without the A extension, so I don't expect the bug y'all want to file to be resolved easily if at all.

bors · 2018-09-01T06:58:23Z

⌛ Testing commit 173c679 with merge e6381a7...

[RISCV] Use lld as the default linker; Enable C extension; Add riscv32imc-unknown-none-elf target The riscv32imc-unknown-none-elf target is intended for soft cores. The riscv32imc target is supported by the following popular soft cores: picorv32: https://github.com/cliffordwolf/picorv32 vexriscv: https://github.com/SpinalHDL/VexRiscv pulp riscy: https://github.com/pulp-platform/riscv pulp zero-riscy: https://github.com/pulp-platform/zero-riscy

bors · 2018-09-01T09:14:50Z

☀️ Test successful - status-appveyor, status-travis
Approved by: japaric
Pushing e6381a7 to master...

rust-highfive assigned varkor Aug 30, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 30, 2018

dvc94ch mentioned this pull request Aug 30, 2018

Completely prebuilt riscv rust toolchain rust-embedded/wg#158

Closed

4 tasks

rust-highfive assigned japaric and unassigned varkor Aug 30, 2018

japaric reviewed Aug 30, 2018

View reviewed changes

dvc94ch force-pushed the riscv branch from 2a5ef56 to 2e10e6a Compare August 30, 2018 14:02

dvc94ch added 3 commits August 30, 2018 16:03

[RISCV] Use lld as the default linker.

f00a952

[RISCV] Enable C extension.

cd862a8

[RISCV] Add riscv32imc-unknown-none-elf target.

8dfd5c3

dvc94ch force-pushed the riscv branch from 2e10e6a to 8dfd5c3 Compare August 30, 2018 14:03

[RISCV] Disable atomics for riscv32imc-unknown-none-elf.

173c679

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 Aug 30, 2018

bors merged commit 173c679 into rust-lang:master Sep 1, 2018

japaric mentioned this pull request Jan 8, 2020

drop cargo-xbuild build dependency iqlusioninc/usbarmory.rs#4

Closed

ducktec mentioned this pull request Nov 23, 2021

"Cannot select" LLVM failure due to unimplemented atomics on msp430 rust-lang/compiler-builtins#441

Closed

davidtwco mentioned this pull request Sep 20, 2023

target tier policy compliance #116004

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RISCV] Use lld as the default linker; Enable C extension; Add riscv32imc-unknown-none-elf target #53822

[RISCV] Use lld as the default linker; Enable C extension; Add riscv32imc-unknown-none-elf target #53822

dvc94ch commented Aug 30, 2018

rust-highfive commented Aug 30, 2018

japaric commented Aug 30, 2018

japaric left a comment

japaric Aug 30, 2018

japaric Aug 30, 2018

japaric Aug 30, 2018

japaric Aug 30, 2018

japaric Aug 30, 2018

dvc94ch Aug 30, 2018

dvc94ch commented Aug 30, 2018

japaric commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

japaric commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

japaric commented Aug 30, 2018

bors commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

bors commented Sep 1, 2018

bors commented Sep 1, 2018

		@@ -25,11 +25,11 @@ pub fn target() -> TargetResult {
		linker_flavor: LinkerFlavor::Ld,

[RISCV] Use lld as the default linker; Enable C extension; Add riscv32imc-unknown-none-elf target #53822

[RISCV] Use lld as the default linker; Enable C extension; Add riscv32imc-unknown-none-elf target #53822

Conversation

dvc94ch commented Aug 30, 2018

rust-highfive commented Aug 30, 2018

japaric commented Aug 30, 2018

japaric left a comment

Choose a reason for hiding this comment

japaric Aug 30, 2018

Choose a reason for hiding this comment

japaric Aug 30, 2018

Choose a reason for hiding this comment

japaric Aug 30, 2018

Choose a reason for hiding this comment

japaric Aug 30, 2018

Choose a reason for hiding this comment

japaric Aug 30, 2018

Choose a reason for hiding this comment

dvc94ch Aug 30, 2018

Choose a reason for hiding this comment

dvc94ch commented Aug 30, 2018

japaric commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

japaric commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

dvc94ch commented Aug 30, 2018

japaric commented Aug 30, 2018

bors commented Aug 30, 2018

hanna-kruppe commented Aug 30, 2018

bors commented Sep 1, 2018

bors commented Sep 1, 2018