[PATCH] D11434: Allow value forwarding past release fences in EarlyCSE
JF Bastien
jfb at chromium.org
Wed Jul 22 16:47:55 PDT 2015
jfb added inline comments.
================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:635
@@ -623,3 +634,3 @@
// We do a trivial form of DSE if there are two stores to the same
// location with no intervening loads. Delete the earlier store.
if (LastStore) {
----------------
I'm not sure I understand why this isn't firing when there's an intervening release fence (`test4` below). I think it could: there was a race anyways, so screw the code and keep one of the store (storing the last value, of course)? Another thread is guaranteed to observe a store to that memory location, but they're racing with each other so the first one isn't guaranteed to ever be observed.
The second store is the more hilarious one to keep, but you could just store its value before the fence?
e.g.:
```
*loc = 42;
std::atomic_thread_fence(std::memory_order_release);
*loc = 0;
```
Becomes:
```
std::atomic_thread_fence(std::memory_order_release);
*loc = 0;
```
Or:
```
*loc = 0;
std::atomic_thread_fence(std::memory_order_release);
```
?
================
Comment at: test/Transforms/EarlyCSE/fence.ll:43
@@ +42,3 @@
+ fence acquire
+ %a2 = load i32, i32* %addr.i, align 4
+ %res = sub i32 %a, %a2
----------------
You can't forward `%a` and eliminate `%a2`, but you could eliminate `%a`, and replace it by `%a2` if there are no intervening uses, no?
http://reviews.llvm.org/D11434
More information about the llvm-commits
mailing list