Can't compile core for msp430 - LLVM ERROR

[chernomor]

$ rustup default nightly-2018-07-07
$ rustup component add rust-src
$ cp -r $(rustc --print sysroot)/lib/rustlib/src/rust/src .
$ cd src/libcore
$ cargo build --target msp430-none-elf
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling core v0.0.0 (file:///tmp/src/libcore)                                                                                                                                           
LLVM ERROR: Cannot select: 0x7f6cb04cf750: i8,ch = AtomicLoad<Volatile LD1[%17]> 0x7f6cb04cfa90:1, 0x7f6cb04cfa90                                                                            
  0x7f6cb04cfa90: i16,ch = load<LD2[%5](dereferenceable)> 0x7f6cb045d440, FrameIndex:i16<2>, undef:i16
    0x7f6cb04cf340: i16 = FrameIndex<2>
    0x7f6cb04cf068: i16 = undef
In function: _ZN4core4sync6atomic11atomic_load17h43092c5ae50dfbfdE
error: Could not compile `core`.                                                                                                                                                             

To learn more, run the command again with --verbose.

$ rustc --version
rustc 1.29.0-nightly (e06c87544 2018-07-06)

Last successful build was on nightly-2018-06-29-x86_64-unknown-linux-gnu rustc 1.28.0-nightly (e3bf634 2018-06-28).

Next builds fails. I check nightly-2018-07-07-x86_64-unknown-linux-gnu unchanged - rustc 1.29.0-nightly (e06c875 2018-07-06) and 1.30.0-nightly (4591a24 2018-09-22)

I see some changes for msp430 between 2018-06-29 and 2018-07-07: https://github.com/rust-lang/rust/compare/e3bf634e0..e06c87544#diff-7fdefb61a42e0c532d40f9742f7bf8cc - may it cause error?

[chernomor]

Commit bbf688a broke building for msp430, it parent (94eb176) good. I build both version and check it.
@japaric can you help?

[japaric]

@awygle @cr1901 @pftbest

Does the msp430 architecture not support atomic loads and stores (and I really mean atomic, not single instruction, those usually lower to atomic fence + load / store)? Or is this just an LLVM bug? I enabled atomic loads / stores on msp430 because @pftbest (iirc) asked for it.

In any case, we can undo these lines to prevent this LLVM error but that would also remove the Atomic*.{load,store} API.

Alternatively, we can set the singlethread codegen option to true. That will turn all atomic operations into volatile operations. IIRC, that's fine for atomic load / stores in presence of interrupts (I might be wrong on this one because a "data synchronization barrier" instruction may still be needed) but unsound for atomic CAS operations in presence of interrupts. If we go down that route then we remove both (a) the possibility of supporting CAS ops in the future (which is probably fine since the ISA doesn't support it) and (b) the possibility of supporting multicore devices with this target (but we can add another target to support those if they ever come into existence).

What do think we should do here?

[awygle]

All loads and stores are atomic on MSP430. There are no fence instructions or atomic instructions. There's no CAS instruction either (as you mention) so we'd have to expand it to turning off interrupts, CASing, then turning them back on (which is kind of silly).

I'm not really clear on what "atomic" vs "volatile" means in the context of Rust, but based on your paragraph above, singlethread sounds fine to me. @pftbest ?

EDIT: all load and store instructions - obviously 32-bit loads/stores are not atomic.

[japaric]

I'm not really clear on what "atomic" vs "volatile" means in the context of Rust

Actually, "volatile" was the wrong term to use as it doesn't describe the right semantics. With singlethread: true LLVM will lower Atomic.load into a compiler_fence plus the load instruction (I think) and with singlethread: false it will lower it into an atomic fence plus the load instruction. The links to the API have more details on the differences between the two kinds of fences.

[awygle]

Okay, great - a compile-time fence is exactly the semantics we want here. My vote is definitely for singlethread: true.

[japaric]

I took at stab at this (setting singlethread to true) and I still get an LLVM error while building core:

LLVM ERROR: Cannot select: t7: i8,ch = AtomicLoad<(volatile load seq_cst 1 from %ir.0)> t6, t2
  t2: i16,ch = CopyFromReg t0, Register:i16 %0
    t1: i16 = Register %0
In function: _ZN67_$LT$core..sync..atomic..AtomicBool$u20$as$u20$core..fmt..Debug$GT$3fmt17h124bcb72c2ea41f0E
error: Could not compile `core`.

It seems that the MSP430 backend doesn't support compiler fences. Minimal example, which can be replicated using nightly:

#![feature(intrinsics)]
#![feature(lang_items)]
#![feature(no_core)]
#![no_core]

extern "rust-intrinsic" {
    fn atomic_singlethreadfence_acq();
}

pub unsafe fn foo(x: *mut u16) -> u16 {
    atomic_singlethreadfence_acq();
    *x
}

#[lang = "copy"]
trait Copy {}

impl Copy for u16 {}

#[lang = "freeze"]
trait Freeze {}

#[lang = "sized"]
trait Sized {}

$ cargo rustc --target msp430-none-elf --release -- --emit=obj
LLVM ERROR: Cannot select: 0x7f6af54571a0: ch = AtomicFence 0x7f6af54703d8, Constant:i16<4>, Constant:i16<0>
  0x7f6af54570d0: i16 = Constant<4>
  0x7f6af5457138: i16 = Constant<0>
In function: _ZN3foo3foo17hdc8504bb86603797E
error: Could not compile `foo`.

Compare this to the ARM Cortex-M backend:

$ cargo objdump --target thumbv7m-none-eabi --release --lib -- -d
foo::foo::he14a1cb0136d5f84:
       0:       00 88   ldrh    r0, [r0]
       2:       70 47   bx      lr

Note that there's no instruction for the fence because this is a compiler fence.

Given this fact the only path left to take here is to remove the Atomic* API on MSP430 until its LLVM backend gains support for compiler fences.