[PATCH] Sink store based on alias analysis

Gerolf Hoflehner ghoflehner at apple.com
Mon Dec 8 11:33:17 PST 2014


Other than nits it LGTM. Thanks for your nice work!

================
Comment at: include/llvm/Analysis/AliasAnalysis.h:515
@@ +514,3 @@
+  /// execution of the specified instructions to mod\ref (according to the
+  /// mode) the value pointed to by Ptr. The instructions to consider are all
+  /// of the instructions in the range of [I1,I2] INCLUSIVE.
----------------
You could clean up the legacy common from "the value pointed to by Ptr" to "the Location" (or \param Location)

================
Comment at: include/llvm/Analysis/AliasAnalysis.h:525
@@ +524,3 @@
+                                 const Instruction &I2, const Value *Ptr,
+                                 uint64_t Size, const ModRefResult mode) {
+    return canInstructionRangeModRef(I1, I2, Location(Ptr, Size), mode);
----------------
It should be Mode (capital M).

================
Comment at: include/llvm/Analysis/AliasAnalysis.h:525
@@ +524,3 @@
+                                 const Instruction &I2, const Value *Ptr,
+                                 uint64_t Size, const ModRefResult mode) {
+    return canInstructionRangeModRef(I1, I2, Location(Ptr, Size), mode);
----------------
mode -> Mode (capital letter) 

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:424
@@ +423,3 @@
+
+    StoreInst *Store1 = (StoreInst *)Inst;
+    BasicBlock *BB0 = Store0->getParent();
----------------
reames wrote:
> Use cast.
There is similar legacy code you might want to clean up too, And you could use auto.

http://reviews.llvm.org/D6420






More information about the llvm-commits mailing list