Document some places where crossbeam tests the limits of the C++ memory model -- or crosses them

[RalfJung]

Thanks to @jeehoonkang for some early feedback.

[ghost]

Thank you!

Now that I think of it, there should probably go an atomic::compiler_fence(Ordering::SeqCst) right after the compare_and_swap operation. Do you agree and if so, can you add it?

[RalfJung]

TBH I have no idea, I don't nearly know enough about this code to answer your question.^^

But my reading of the comment (above the stuff I added) is that this fence is deliberately not added because in x86 assembly, it is not needed, but the compiler is not smart enough to optimize this correctly. Of course that doesn't mean the compiler won't do something stupid here, but unfortunately SC accesses (mixed with other accesses to the same location) are amongst the least understood parts of the C++ concurrency model...

Let's see what @jeehoonkang has to say.

[jeehoonkang]

Yeah, I think the motivation here is avoiding MFENCE in the generated assembly while achieving strong enough synchornization with LOCK CMPXCHG. I believe the code should really be written in inline assemby, rather than some C++ code that will be compiled to LOCK CMPXCHG, but unfortunately we cannot do that in the stable Rust.

To summarize my stance, there are two hacks in play:

• We're using x86 LOCK CMPXCHG instead of C++ fence(SeqCst) for avoiding the expensive MFENCE. It's correctness is not yet theoretically investigated, but it seems working.
• In fact, we're using C++ update instead of x86 LOCK CMPXCHG because inline assembly is currently Nightly only. To be snobbish, we should check whether the generated assembly indeed has LOCK CMPXCHG.

@RalfJung I just realized this point. Sorry to have you work twice, but if you agree with my explanation, would you please document it as a comment?

[RalfJung]

The first part isn't a hack though, is it? It is quite clear from the x86/TSO model that a CAS has the effect of a fence.

So the only hack here is to pretend we are working in assembly while we are not -- to rely on how CAS is codegen'd, and to hope that either this is correct in the C++ model as well or else LLVM does not notice what we are doing here.

[ghost]

Exactly - we'd ideally use inline assembly here but can't on stable Rust. The CAS will compile into the desired lock cmpxchg instruction, and it is indeed an ugly hack. :)

I think it'd be a good idea to put a compiler fence after the CAS so that it becomes a real fence from the processor's standpoint and from the compiler's standpoint.

[RalfJung]

I extended the comment to mention inline assembly.

Do we have compiler fences on stable Rust...?

[ghost]

We do, since 1.21: https://doc.rust-lang.org/std/sync/atomic/fn.compiler_fence.html

[RalfJung]

I added a compiler fence after the CAS.

[ghost]

@RalfJung I like how precise and cautious your comments are :)

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push