[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

[Daniel Berlin via llvm-commits]

dberlin added inline comments.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:106
@@ +105,3 @@
+
+// This hash matches the most common things isSameOperationAs checks. It must
+// always be a subset or equal to isSameOperationAs for everything to function
----------------
To answer the original question, i'm checking to see about lambdas.

No generic Instruction hasher exists that covers precisely the set of stuff we care about. GVN does what newgvn does, which is it defines its own expression class, shoves operands in those classes, and calls hash_combine on the whole shebang:


```
friend hash_code hash_value(const Expression &Value) {
    return hash_combine(
        Value.opcode, Value.type,
        hash_combine_range(Value.varargs.begin(), Value.varargs.end()));
  }


```
The default hasher will end up hashing the pointer values themselves, which is what gvn wanted in this case.

It is not useful, however, for hashing "parts of operations you care about", or "do two operations look pretty similar".


Past that, i did not change the things included in the hash to ensure consistency of what is optimized with the existing code.


================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:117
@@ +116,3 @@
+  }
+};
+
----------------
It is not, it goes straight into comparing all sorts of fields/data.



http://reviews.llvm.org/D8688