[clang] [llvm] [IR] Remove volatile from nosync (PR #194391)

[Nikita Popov via cfe-commits]

nikic wrote:

Okay, I should probably have provided more context here. I think there are a few things to keep in mind here:

 * LLVM's IR model does not allow volatile to perform synchronization (we have a pretty precise definition of what volatile is allowed to do, and this is explicitly excluded). Maybe more importantly, the LLVM *implementation* does not treat volatile as synchronizing (in particular in terms of AA handling). The effects of volatile are strictly on the location being accessed and on inaccessible memory. Volatile does not prevent reordering of other accesses around it. So using a volatile memory access to spell a memory barrier is just not something that you can do, *both* de jure and de facto.
 * The nosync attribute is currently essentially unused. I want to make use of (absence of) nosync to address issues like https://github.com/llvm/llvm-project/issues/64188. For that, the only kind of "synchronization" we care about are stronger-than-monotonic atomics, as only they introduce reordering barriers for unrelated memory locations. Treating volatile operations as "synchronizing" in this sense is very undesirable, and entirely at odds with how isolated volatile operations are currently treated. Now, of course, I could instead leave nosync alone and introduce a new nofence attribute with the new semantics, but that just adds more confusion and complexity. I'd rather reuse the existing, largely unused, nosync attribute for this purpose.
 * The only other real use nosync has right now is in conjunction with nofree, part of our experimental (not enabled by default) dereferenceable-at-point semantics. I plan to untangle the relationship between nofree/nosync in a separate change, so if we really, really wanted we could define that volatile ops are nosync but not nofree, though I'm very skeptical that this is a sensible thing to do.
 * I think part of the problem here is that the current definition of nosync is really fuzzy -- the current definition talks about "not communicate (synchronize)". With that wording, excluding volatile ops is scary -- heck, even unordered/monotonic atomics can "communicate" with other threads, they just can't synchronize, so using these terms as if they were interchangeable is odd. I've now pushed a change to instead define "nosync" in terms of *synchronizes-with*, which I think makes it more precise what this attribute means (and is also exactly what I want in terms of semantics from this attribute). Does this definition resolve the concern? I think it should be uncontroversial that volatile does not synchronize-with.

The tl;dr is that I don't think the current semantics of the nosync attribute is really useful for anything (and, as a corollary, not actually used for anything meaningful), and I'd like to change it in a direction where it can actually serve a useful purpose. For that, I believe it's necessary to align nosync more closely to "no synchronizes-with", and in particular exclude volatile accesses.

https://github.com/llvm/llvm-project/pull/194391