Permit (Release, Acquire) ordering for compare_exchange[_weak] and add appropriate compiler intrinsic

[oliver-giersch]

This will probably require further discussion, but according to my research the current rule of disallowing (Release, Acquire) as memory ordering for the atomic compare_exchange[_weak] primitives is unnecessarily strict and not actually required by the memory model.
Here is a link to a previous discussion on that topic and here a link to a question and answer I've posted to the llvm-dev mailing list (my follow-up was unfortunately not answered). According to that answer, LLVM currently does not consider Acquire to be a stronger ordering than Release (source code) and apparently there are plans to drop the "relative strength" requirement altogether, since it is basically not really well-defined.

Here is an example using clang on ARM that shows different assembly is generated when (Release, Acquire) is used instead of the next best approximation (AcqRel, Acquire).

[rust-highfive]

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

[tmiasko]

Thanks for working on this.

The produced assembly doesn't respect acquire ordering on the failure (https://bugs.llvm.org/show_bug.cgi?id=33332, #68464).

[oliver-giersch]

Thanks for working on this.

The produced assembly doesn't respect acquire ordering on the failure (https://bugs.llvm.org/show_bug.cgi?id=33332, #68464).

Thanks for bringing up these issues, I wasn't aware at all and it hadn't come in the course of my research. I am also not proficient enough in reading ARM assembly to have spotted the miscompilation myself.

[bors]

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC-zz]

@oliver-giersch we do not support merge commits in the rust repository. Would recommend rolling it back and doing a rebase instead.

[oliver-giersch]

Would it be a sensible compromise to leave the intrinsics in place but disallow using the (Release, Acquire) order for CAS operations, until there is progress on the LLVM codegen bugs are resolved?

[bors]

☔ The latest upstream changes (presumably #75797) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC-zz]

@oliver-giersch any updates on this? (also if you can, do resolve the conflicts)

[oliver-giersch]

@Dylan-DPC I would prefer if I could get some feedback on the matter in principle before resolving further conflicts, specifically:

1. Should this ordering combination be permitted at all? In my opinion the memory model allows it, but I know from previous discussion that this issue is somewhat controversial
2. Given that LLVM currently does not compile this ordering combination correctly on some platforms, should perhaps for now only the intrinsics be added but not publicly exposed?

[cramertj]

Given that LLVM currently does not compile this ordering combination correctly on some platforms, should perhaps for now only the intrinsics be added but not publicly exposed?

What would be the goal of adding the intrinsics if they aren't to be used?

[oliver-giersch]

@cramertj my idea was to enable their use at a later point if and when LLVM fixes its codegen, which I assume has to happen at some point, given that C++20 relaxes some ordering restrictions.

[crlf0710]

Triage: There's still merge conflict. But it seems the author is also seeking for some discussions.

@oliver-giersch Could you open a thread on Zulip or file a T-lang MCP for such a change? This pr should block on such an MCP before it could proceed.

[JohnCSimon]

Ping from triage:
@oliver-giersch - Thanks for your contribution, but unfortunately I'll have to close this due to inactivity, please feel free to reopen when you are ready to proceed.