riscv-rt: partial compatibility with RV32E

[romancardenas]

Avoids using x16+ registers in assembly code. This is necessary to provide compatibility with RV32E.
Note that there is still work to do. Namely, the trap frame must adapt to the target base instruction set.

Solves #189

Related: #239

[rmsyn]

Would it be a good idea to order these fields by there x* name (similar to the spec)?

Also, maybe we use coniditional compilation to not include the x16-x31 registers for RV32E builds?

Mostly thinking about downstream users that may use a pointer to the TrapFrame, which they may or may not try to map to memory layout.

It may not be necessary, but could be a helpful reminder that those registers don't exist on RV32E platforms.

[romancardenas]

I'd try to avoid changing the order of the attributes in case users use C or assembly code that relies on the current order.

I'm working on feature-gating registers for RVE32. It's a bit tricky, as we need to add "trash" padding to ensure that the stack is 16-byte aligned. I'll open a PR shortly with all this.

[rmsyn]

I'd try to avoid changing the order of the attributes in case users use C or assembly code that relies on the current order.

I guess that's also part of what I'm asking, "should we intentionally break downstream users compiling RV32E?" Especially regarding not providing them in-memory registers that don't exist on the platform.

For non-RV32E users, I agree with you, we should avoid breaking them without a good reason.

I'm working on feature-gating registers for RVE32. It's a bit tricky, as we need to add "trash" padding to ensure that the stack is 16-byte aligned. I'll open a PR shortly with all this.

Sounds good, I'll keep an eye out for it.

Maybe for the missing registers on RV32E targets:

pub struct TrapFrame {
    #[cfg(not(rv32e))]
    pub a6: usize,
    #[cfg(rv32e)]
    _a6: usize,
    ...
}

[romancardenas]

I've been working on a complete riscv-target-parser crate to help us in the build scripts. Also, I think it is time to stop using the riscv, riscv32, and riscv64 cfg flags and just use target_arch.

My main challenge now is configuring riscv-rt-macros according to flags triggered in riscv-rt. I don't know how to do this without features, but I will read elsewhere and see if there is any other approach. I don't love letting users activate a riscve feature regardless of the target.

[rmsyn]

My main challenge now is configuring riscv-rt-macros according to flags triggered in riscv-rt. I don't know how to do this without features

If this specifically about RV32E stuff, maybe a combination of target_arch and target_feature flags in a #[cfg(...)]? Although, it looks like there are some restrictions on using target_feature on riscv* targets.

I would be happy to help out, if you have some work-in-progress code you would like extra eyes on.

[romancardenas]

This is the best way I could think of. I tried forwarding environment variables from build scripts to no avail, as procedural macros are compiled separately from regular crates. The macro code can be squeezed a bit, but more or less this is it.

[rmsyn]

This is the best way I could think of. I tried forwarding environment variables from build scripts to no avail, as procedural macros are compiled separately from regular crates. The macro code can be squeezed a bit, but more or less this is it.

Yeah, I tried using conditional compilation with target_feature = "i" + target_feature = "e", but it doesn't look like base ISA are available.

If target_feature = "i" and target_feature = "e" were available, we could use conditional compilation to define a core_interrupt_inner the four variants we care about, i.e. riscv32e, riscv32i, riscv64e, riscv64i. Then we could have the single core_interrupt macro function wrapping the unsafe inner functions.

I can look at the rustc/cargo source, and see what it takes to add base ISA as conditional compilation attribute. It may make more sense for the base ISA stuff to go under the instruction_set attribute (currently only has ARM in the docs).

The changes here LGTM.

Edit: something like this is what I was thinking: rv32e-compat-asm...rmsyn:riscv:rv32e-compat-asm

[romancardenas]

I tried something like that, but I found out that the procedural macros are compiled for the host, not the target. Thus, the cfg(target_arch = "riscv32") does not work unless your host machine is a RV32

[rmsyn]

I tried something like that, but I found out that the procedural macros are compiled for the host, not the target. Thus, the cfg(target_arch = "riscv32") does not work unless your host machine is a RV32

What about replacing them with the cfg! macro, which is evaluated at run-time instead of build-time?

Something like: rmsyn/riscv@rust-embedded:riscv:rv32e-compat-asm...rv32e-compat-asm