[PATCH] D11434: Allow value forwarding past release fences in EarlyCSE
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 26 17:29:00 PDT 2015
reames added inline comments.
================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:617
@@ +616,3 @@
+ // A release fence requires that all stores complete before it, but does
+ // not prevent the reordering of following loads 'before' the fence. As a
+ // result, we don't need to consider it as writing to memory and don't need
----------------
jfb wrote:
> reames wrote:
> > jfb wrote:
> > > Getting re-acquainted with the code: it doesn't seem like this change exercises a test?
> > Huh? Not sure what you're asking. The first two tests in the included file cover cases this triggers.
> True, they check that store/fence/load and store/fence/store are handled properly with release.
>
> What I mean is: the code changes, but all of the tests are *exactly the same*. If code changes but tests remains the same, then what changed? :-)
I'm still not clear what you're trying to ask. We remove a load from both test1 and test2 which is not removed without the patch. (Well, at the time I wrote it. Haven't checked today, but I don't see any obvious new additions.) Can you clarify?
http://reviews.llvm.org/D11434
More information about the llvm-commits
mailing list