[PATCH] Sink store based on alias analysis

[hfinkel at anl.gov]

================
Comment at: include/llvm/Analysis/AliasAnalysis.h:518
@@ +517,3 @@
+  /// I1 and I2 must be in the same basic block.
+  bool canInstructionRangeRefModify(const Instruction &I1,
+                                const Instruction &I2, const Location &Loc,
----------------
I agree, this seems like a useful generalization. 'RefModify' seems odd to me, however. We use ModRef for most related things, how about:
  canInstructionRangeModRef

================
Comment at: lib/Transforms/IPO/ArgumentPromotion.cpp:558
@@ -558,1 +557,3 @@
+    if (AA.canInstructionRangeRefModify(BB->front(), *Load, Loc, 
+      AliasAnalysis::Mod))
       return false;  // Pointer is invalidated!
----------------
Indentation seems off here.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:420
@@ -426,11 +419,3 @@
 
-    // Only move loads if they are used in the block.
-    if (isStoreSinkBarrier(Inst))
-      break;
-    if (isa<StoreInst>(Inst)) {
-      AliasAnalysis::Location LocSI = AA->getLocation(SI);
-      AliasAnalysis::Location LocInst = AA->getLocation((StoreInst *)Inst);
-      if (AA->isMustAlias(LocSI, LocInst)) {
-        I = (StoreInst *)Inst;
-        break;
-      }
+    if (!isa<StoreInst>(Inst) || Inst->isUsedOutsideOfBlock(BB1))
+       continue;
----------------
Why did you add the Inst->isUsedOutsideOfBlock check. We know that Inst is a store, and stores can't be 'used' anywhere.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:423
@@ +422,3 @@
+
+    StoreInst *Store1 = dyn_cast<StoreInst>(Inst);
+    BasicBlock *BB0 = Store0->getParent();
----------------
We know that Inst is a store, so this should be cast, not dyn_cast.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:429
@@ +428,3 @@
+    if (AA->isMustAlias(Loc0, Loc1) && Store0->isSameOperationAs(Store1)
+      && !isStoreSinkBarrierInRange(*Store1, BB1->back(), Store1) &&
+      !isStoreSinkBarrierInRange(*Store0, BB0->back(), Store0)) {
----------------
I think we prefer the logical operator before the line break, not after. Can you move the first '&&' to the line above?

http://reviews.llvm.org/D6420