[PATCH] D22001: [DSE] Remove dead stores in end blocks containing fence

[Philip Reames via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/ comments addressed.


================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:774
@@ -773,1 +773,3 @@
 
+    // We can remove the dead stores in the end basic block, irrespective of the
+    // fence and its ordering (release/acquire/seq_cst). Fences only constraints
----------------
Minor: As JF's question points out, mentioning the end block in the comment implies that the fact it's an end block is important for the reasoning.  The important part is that the fence doesn't make a dead store non-dead. The fact that the surrounding code determines deadness only in end blocks is irrelevant to the key logic your documenting.  

================
Comment at: test/Transforms/DeadStoreElimination/fence.ll:50
@@ +49,3 @@
+
+; We DSE stack alloc'ed pointer operands, byval attributes, and calloc-like operations at end blocks (contains no successors) irrespective of fence ordering.
+; The store to %addr.i can be removed since it is a byval attribute
----------------
Comment is again confusing.  The key thing to highlight is that a fence doesn't make an otherwise thread local store visible.


http://reviews.llvm.org/D22001