UB due to lack of a full fence in full_fence (on x86) about event-listener

[RalfJung]

I don't think it is allowed to completely remove independent SC operations

LLVM certainly does not agree with your reasoning: this becomes a NOP.

pub fn test() {
    let x = AtomicUsize::new(0);
    x.load(Ordering::SeqCst);
}

Stores also get removed. I think they just didn't implement the pass that removes dead RMWs.

[notgull]

The MSRV of the smol-rs project previously did not allow for inline assembly. See smol-rs/smol#244. But now it does, so we should probably bump the MSRV so we can directly use the code that we want.

[taiki-e]

Filed #71 to resolve this.

why doesn't this use inline assembly?

When this crate was first released, inline assembly was not stable. And for a long time after that, we used MSRV that older than 1.59.

Since we have recently raised the MSRV of some smol-rs crates to 1.63, I think this is an appropriate time to address this issue.

and the myth that no sane compiler is going to optimize atomics is a myth. However, I admit I don't know enough about this specific case to say whether this is a risk for this crate.

The C++20 memory model states that the SC operation "establish a single total modification order of all atomic operations that are so tagged". https://en.cppreference.com/w/cpp/atomic/memory_order#Sequentially-consistent_ordering

While it may be possible to "merge two atomic operations when there are only pure operations in the middle" as described in the article you linked to, I don't think it is allowed to completely remove independent SC operations (unless the compiler can prove that there are never any SC operations that could be synchronized with it by inspecting all programs). (It may be possible to replace SC CAS with SC fence or other SC RMW, but it doesn't lose the property we need here.)

So, unless the C++ memory model changes the property mentioned above in the future, I expect that no wrong code will be generated here, but inline assembly is the "definitely correct" choice here, and I agree that using inline assembly is definitely preferable.

[RalfJung]

The MSRV of the smol-rs project previously did not allow for inline assembly. See smol-rs/smol#244. But now it does, so we should probably bump the MSRV so we can directly use the code that we want.

Ah, if it's just an MSRV issue that is great! And thanks @taiki-e for getting the ball rolling with that PR. :)

If the lock cmpxchg can be faster than mfence it might also be worth trying to get LLVM to generate the former instead of the latter for SC fences -- then all code would benefit.

The C++20 memory model states that the SC operation "establish a single total modification order of all atomic operations that are so tagged". https://en.cppreference.com/w/cpp/atomic/memory_order#Sequentially-consistent_ordering

Yeah, but the way that order interacts with the other orders isn't always entirely how one would expect, I think... so it's certainly not obvious to me that compilers are not allowed to drop the access. In particular this order AFAIK does not participate when determining synchronizes-with edges -- but an SC fence has the effects of a release-acquire fence and can hence establish synchronizes-with edges.

You have to study the formalizations of this model, I don't think the English prose can be precise enough for subtle questions like this.

[taiki-e]

Oh, that is interesting. I can reproduce it, even with a very old rustc, even if I used compiler_fence around SC load/store. https://godbolt.org/z/xsvG5Ke58