Stream: t-lang/wg-unsafe-code-guidelines

Topic: atomic intrinsic design


RalfJ (Jul 26 2019 at 16:55, on Zulip):

@gnzlbg

FWIW I never understood why we need AtomicXY types, instead of having ptr.atomic_load(...) methods.

Yeah I wondered the same. It's very annoying that e.g. AtomicCell has to transmute (u8, u16) to u32 to implement atomic accesses...

gnzlbg (Jul 26 2019 at 16:56, on Zulip):

I think it would be better to have intrinsics taking pointers that can be used to build the types.

RalfJ (Jul 26 2019 at 16:58, on Zulip):

:+1:

Lokathor (Jul 27 2019 at 07:26, on Zulip):

I think it was an attempt to try and limit the types that you'd use atomic with, in the same way that right now you can volatile load and store any pointer but it only works without tearing at specific sizes, per arch

Lokathor (Jul 27 2019 at 07:27, on Zulip):

Also, it makes statics way easier

RalfJ (Jul 27 2019 at 07:37, on Zulip):

what is the interaction with statics?

Lokathor (Jul 27 2019 at 08:56, on Zulip):

so, when you have a static value you declare it

pub static SDL_ACTIVE: AtomicBool = AtomicBool::new(false);

or similar. Then every thread sees it and can safely call load and store all that. If there wasn't a dedicated type for this you'd have to do a whole song and dance to make an atomically editable bool. It'd be a static mut, and then you'd pray to Ferris and unsafely take a &mut to it in the hope that no other thread was doing that right then, call atomic_whatever, and it'd just be a lot more error prone.

RalfJ (Jul 27 2019 at 09:04, on Zulip):

@Lokathor we are only talking about the internal intrinsics here that are used to implement these types; I agree that AtomicBool makes sense as a user-facing type.

RalfJ (Jul 27 2019 at 09:05, on Zulip):

however, it should be possible to do atomic accesess on a *mut (u8, u16). currently you have to cast to *mut u32 which is (a) hacky and (b) raises concerns about padding.

rkruppe (Jul 27 2019 at 09:15, on Zulip):

I don't understand how atomic_whatever::<(u8, u16)>(ptr, ...) would help with anything (it actually exists internally!). The hardware only provides atomic accesses to (naturally aligned) 8/16/32/... bit chunks. If you want to use the 32 bit atomics with a honest-to-god (u8, u16), you have to worry about the padding (and alignment) anyway at some level because it's a fundamental mismatch.

Lokathor (Jul 27 2019 at 09:21, on Zulip):

ah! for internals, okay

RalfJ (Jul 27 2019 at 09:54, on Zulip):

@rkruppe at least for loads and stores, there's no good reason I should have to transmute my (u8, u16) to u32 before storing it

rkruppe (Jul 27 2019 at 10:03, on Zulip):

Fair, though there's still alignment concerns -- you can't load/store with arbitrary &mut (u8, u16), it has to be aligned to 4 bytes. In fact, I recall the existing unstable intrinsics could be instantiated with (u8, u16) but rustc would put the wrong alignment on the LLVM intrinsic.

Славик Бут (Jul 27 2019 at 10:17, on Zulip):

(deleted)

RalfJ (Jul 27 2019 at 10:24, on Zulip):

hm, yeah, I didnt think about alignment for a sec and there it got me again

gnzlbg (Jul 27 2019 at 11:46, on Zulip):

atomic_relaxed_load_u32(ptr: *const u32) -> MaybeUninit<u32> would just require ptr to be aligned to a 4-byte boundary

gnzlbg (Jul 27 2019 at 11:47, on Zulip):

So if you have:

let x: (u8, u16);
let y = unsafe {
    atomic_relaxed_load_u32(&x as *const _ as *const _)
};

and if the pointer isn't properly aligned the behavior is undefined. clippy will actually yell at this code, warning that you are casting pointers with different alignment requirements.

gnzlbg (Jul 27 2019 at 11:49, on Zulip):

I don't see how alignment is any more problematic than, e.g., size, with such an API. For example:

let x: u8;
let y = unsafe {
    atomic_relaxed_load_u32(&x as *const _ as *const _)
};

would read out-of-bounds, which is also UB.

Last update: Nov 19 2019 at 18:00UTC