[PATCH] Sink store based on alias analysis

Philip Reames listmail at philipreames.com
Wed Dec 3 09:57:07 PST 2014


================
Comment at: include/llvm/Analysis/AliasAnalysis.h:509
@@ -508,3 +508,3 @@
   /// canBasicBlockModify - A convenience wrapper.
   bool canBasicBlockModify(const BasicBlock &BB, const Value *P, uint64_t Size){
     return canBasicBlockModify(BB, Location(P, Size));
----------------
It would be good to keep this interface consistent with the one you're changing.  Leaving wrappers for the old interface names would be fine, or converting the BasicBlock versions to the ModRef form.  I'd slightly prefer leaving convenience wrappers under the old names.  

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:421
@@ -426,11 +420,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))
+       continue;
----------------
Rather than stopping on the first readwrite call, this function will now rescan the end of the basic block for each store in the basic block.  That seems mildly expensive: O(N^2) in the number of instructions in the BB.  Have you considered the implications for compile time?

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:424
@@ +423,3 @@
+
+    StoreInst *Store1 = (StoreInst *)Inst;
+    BasicBlock *BB0 = Store0->getParent();
----------------
Use cast.

http://reviews.llvm.org/D6420






More information about the llvm-commits mailing list