Alternative reprs for bitfields

[jonathanpallant]

Doing a volatile write on a struct type causes LLVM to emit some annoying loads/stores using the stack. This seems to be because the volatile operation gets converted to a volatile memcpy, and both the source (RAM) and the destination (MMIO) get treated as volatile.

I suspect that selecting repr(transparent) instead of repr(C) would fix this. Is it possible to have a way to change the default repr?

Basically given:

/// Interrupt Register
#[bitbybit::bitfield(u32, debug, default = 0)]
pub struct Ir {
	#[bit(4, rw)]
	pub cr_interrupt: [bool; 4],
	#[bit(0, rw)]
	pub mr_interrupt: [bool; 4],
}

I'd like to change the output from:

#[repr(C)]
/// Interrupt Register
pub struct Ir {
    raw_value: u32,
}

to:

#[repr(transparent)]
/// Interrupt Register
pub struct Ir {
    raw_value: u32,
}
``

[jonathanpallant]

See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/rustc.20emitting.20redundant.20load.2Fstores.20at.20opt-level.3Ds/with/603507275 for discussions.

[danlehmann]

Interesting find! Let me try switching this. I have a big project where I'm using this with volatile a lot and I wonder if that is going to make a difference there.

[danlehmann]

Not sure what the public API implications of this would be

[jonathanpallant]

Not sure what the public API implications of this would be

Annotated types would change, which is a breaking change on those types.

So this might need to be version 3? Or it could be opt-in, with repr = "transparent"?

[danlehmann]

Interestingly, I noticed it changes some clippy rules. This now requires allow(non_camel_case_types) (or a rename):

#[bitfield(u32, default = 0)]
pub struct MPU_RBAR {
}

[danlehmann]

Tested this on a big repo. Tiny binary size wins all over the place, so I'll definitely want to fix this. Looking into a proper fix that keeps backwards compatibility in mind.

Thanks for filing @jonathanpallant !

[flaviojs]

Personally I would go for #[bitfield(u32, repr(transparent))],
defaulting to repr(C) in 2.x and repr(transparent) in 3.x.

[scottmcm]

Doing a volatile write on a struct type causes LLVM to emit some annoying loads/stores using the stack.

Technically, this is probably from something being https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html#variant.Memory instead of https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html#variant.Scalar -- in general, it seems like the volatile implementations in rustc might just need more thought for things that don't get scalar reprs, since for example how volatile loads are implemented is arguably also just wrong for what LLVM wants (see rust-lang/rust#157127)

So a couple thoughts:

• you probably want to use a scalar (via transparent) for things short term if you want to do volatile stuff
• the rustc code here can probably be improved without too much trouble which might make a bunch of the simple cases get better without actually needing library changes
• but also the rules for what volatile does are quite ill-defined right now (Add splitting caveats to {read,write}_volatile rust-lang/rust#156931), so if you want meaningful volatile on non-scalars you probably want to work with opsem to get actual guarantees