c2rust icon indicating copy to clipboard operation
c2rust copied to clipboard

Conversion of atomic builtins needlessly uses Rust intrinsics

Open chrysn opened this issue 3 years ago • 2 comments

Example

Compiling this function:

static inline __attribute__((always_inline))
void cpu_reg_disable_bits(volatile uint32_t *reg, uint32_t mask)
{
    __atomic_fetch_and(reg, ~mask, __ATOMIC_RELAXED);
}

produces this Rust code:

#[inline(always)]
unsafe extern "C" fn cpu_reg_disable_bits(mut reg: *mut uint32_t,
                                          mut mask: uint32_t) {
    ::core::intrinsics::atomic_and_relaxed(reg, !mask);
}

This is correct, but requires the core_intrinsics on the produced code (which is otherwise usable on stable Rust).

Code orientation

The relevant code is produced in c2rust-transpile/src/translator/builtins.rs around line 530; note that one doesn't find precisely "atomic_and_relaxed" because the name is assembled from the operation and the ordering.

Replacement

The AtomicXX family of types has a fetch_and, which are suitable replacements -- at least if the type that is being operated on is part of the family.

I'm holding off on fixing this while #364 is hot; I plan to come back to this later and can probably come up with a PR then.

chrysn avatar Mar 08 '22 22:03 chrysn

Could you sketch what the above code would look like using the types from std::sync::atomic or core::sync::atomic? I'm not sure if you'd use AtomicPtr, AtomicU32, or both to accomplish the same as the present translation.

thedataking avatar Mar 09 '22 17:03 thedataking

I have a workaround currently in place for the concrete u32 case, where the module is a drop-in replacement for ::core::intrinsics:

fn atomic_and_relaxed(dst: *mut u32, src: u32) -> u32 {
    let actual_atomic = unsafe { &*(dst as *mut AtomicU32) };
    actual_atomic.fetch_and(src, Ordering::Relaxed)
}

I think that the translator has all the information available to see that the uint32_t of the example is an alias to u32. Then it could perform a lookup on the type (u32AtomicU32) and the access (__atomic_fetch_andfetch_and) and thus directly build the atomic operation in place.

chrysn avatar Mar 09 '22 17:03 chrysn