[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