Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LLVM's unordered intrinsic to Rust. #19835

Merged
merged 1 commit into from
Jan 3, 2015

Conversation

pythonesque
Copy link
Contributor

This corresponds to the JMM memory model's non-volatile reads and writes to shared variables. It provides fairly weak guarantees, but prevents UB (specifically, you will never see a value that was not written at some point to the provided location). It is not part of the C++ memory model and is only legal to provide to LLVM for loads and stores (not fences, atomicrmw, etc.).

Valid uses of this ordering are things like racy counters where you don't care about the operation actually being atomic, just want to avoid UB. It cannot be used for synchronization without additional memory barriers since unordered loads and stores may be reordered freely by the optimizer (this is the main way it differs from relaxed).

Because it is new to Rust and it provides so few guarantees, for now only the intrinsic is provided--this patch doesn't add it to any of the higher-level atomic wrappers.

@pythonesque
Copy link
Contributor Author

The more I read about this the less useful it seems... if it's a version of Monotonic that can be freely reordered, I doubt it's useful for much if you already know a variable is shared.

@pythonesque
Copy link
Contributor Author

@thestinger The problem is that I'm not even sure it's useful for that (or at least, notably more useful than Monotonic). From the LLVM atomics documentation on Monotonic:

In terms of the optimizer, this can be treated as a read+write on the relevant memory location (and alias analysis will take advantage of that). In addition, it is legal to reorder non-atomic and Unordered loads around Monotonic loads. CSE/DSE and a few other optimizations are allowed, but Monotonic operations are unlikely to be used in ways which would make those optimizations useful.

...

Code generation is essentially the same as that for unordered for loads and stores. No fences are required. cmpxchg and atomicrmw are required to appear as a single operation.

Similar documentation elsewhere in LLVM suggests that unordered loads can also probably be reordered around everything else, with the possible exception of explicit fences (and it's not even completely clear to me that unordered loads and stores respect those). So I am not sure whether it really buys us anything over Relaxed--it seems like it's basically got incorrect semantics for anything useful if you actually are sharing memory, and only exists because it would prohibit a lot of optimizations if Java used Monotonic everywhere.

@Gankra
Copy link
Contributor

Gankra commented Jan 2, 2015

@pythonesque Do you still think this has value? Do you need a reviewer?

@pythonesque
Copy link
Contributor Author

@gankro I think it still has value (for the same reason it has value in Java) and I don't think it would be harmful to add to Rust. I just have fewer immediate usecases for it.

@Gankra
Copy link
Contributor

Gankra commented Jan 2, 2015

@thestinger are you willing to review?

@thestinger
Copy link
Contributor

I think it needs a feasible use case before it should be merged.

@pythonesque
Copy link
Contributor Author

@thestinger I think certain operations are safe with it where the value can't ultimately affect correctness (e.g. while writing a spinlock, doing an unordered load while spinning to see if it's worth doing the CAS, or the aforementioned racy counters). In practice I suspect a monotonic load will have exactly the same codegen there (up to operation reordering that you likely don't want), plus saner behavior overall (and if you want more specific fast behavior on particular architectures you probably have to drop into assembly anyway). So really the only usecase I see is implementing something like a JVM bytecode interpreter, where you'll have non-Rust code running in a language that can't enforce Rust's no data race guarantee. TBH, I don't know if that's a useful enough case. If you don't think so, feel free to close this.

@thestinger
Copy link
Contributor

@pythonesque: I think it's enough that the code generator can generate better code in practice, considering that it's a low-level primitive. It can be implemented in terms of _relaxed so there's no real increase in the implementation burden for a new compiler backend.

bors added a commit that referenced this pull request Jan 3, 2015
…estinger

This corresponds to the JMM memory model's non-volatile reads and writes to shared variables.  It provides fairly weak guarantees, but prevents UB (specifically, you will never see a value that was not written _at some point_ to the provided location).  It is not part of the C++ memory model and is only legal to provide to LLVM for loads and stores (not fences, atomicrmw, etc.).

Valid uses of this ordering are things like racy counters where you don't care about the operation actually being atomic, just want to avoid UB.  It cannot be used for synchronization without additional memory barriers since unordered loads and stores may be reordered freely by the optimizer (this is the main way it differs from relaxed).

Because it is new to Rust and it provides so few guarantees, for now only the intrinsic is provided--this patch doesn't add it to any of the higher-level atomic wrappers.
@emberian
Copy link
Member

emberian commented Jan 3, 2015

@thestinger what ended up being the concrete usecase for this? just as an optimization for "speculative" locking?

@bors bors merged commit ccd88c5 into rust-lang:master Jan 3, 2015
@thestinger
Copy link
Contributor

@cmr: It's theoretically useful for speeding up approximate statistics via counters which is good enough for me.

@thestinger
Copy link
Contributor

i.e. LLVM is a lot more free to move around the counter increments or do something like merging 3 writes into one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants