[PATCH] D8313: Refactor commoning of target specific load/store intrinsics in EarlyCSE

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: include/llvm/Analysis/LoadStoreSite.h:99
@@ +98,3 @@
+  /// type of instruction.  Given a load instruction, a compatible store
+  /// instruction is the one that is a mirror of the load instruction.
+  bool isCompatible(const Value *Other) const {
----------------
hfinkel wrote:
> reames wrote:
> > This API feels really awkward.  For now, I'll ask that you pull this function out of the helper class and make it a static function in EarlyCSE.  That at least localizes the awkwardness to the place it's used.  
> Philip, why do you feel this is awkward? It seems that if the point of the abstraction is to hide the differentiation between regular load/stores and related target intrinsics. As such, it seems that this is an important part of the abstraction.
Er, I've forgotten it's been so long since I looked at this.  This was a minor comment and can be safely ignored.  This can be addressed post commit if needed; I really don't want to block submission of this given how long it's been outstanding.  


http://reviews.llvm.org/D8313