[PATCH] D23032: Rewrite the use optimizer to be less memory intensive and 50% faster.Fixes PR28670

[Sebastian Pop via llvm-commits]

sebpop added a subscriber: sebpop.

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1116
@@ +1115,3 @@
+      return CS.getCalledValue() == Other.CS.getCalledValue();
+    else
+      return Loc == Other.Loc;
----------------
No need for "else" after return.

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1139
@@ +1138,3 @@
+                              MLOC.getCS().getCalledValue()));
+    else
+      return hash_combine(
----------------
Remove "else".

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1246
@@ +1245,3 @@
+    auto &LocInfo = LocStackInfo[UseMLOC];
+    // If the pop epoch changed, if means we've removed stuff from top of
+    // stack due to changing blocks. We may have to reset the lower bound or
----------------
s/if means/it means/

================
Comment at: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp:1152
@@ +1151,3 @@
+/// is a batch walker that touches everything, it does not operate like the
+/// other walkers.  This walker is basically performing a top-down SSA renaming
+/// pass, where the version stack is used as the cache.  This enables it to be
----------------
Would it be possible to merge the UseOptimizer with the renamePass?

================
Comment at: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp:1284
@@ +1283,3 @@
+      DEBUG(dbgs() << "We are being asked to check up to "
+                   << UpperBound - LocInfo.LowerBound
+                   << " loads and stores, so we didn't.\n");
----------------
This message needs some rewording.

================
Comment at: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp:1462
@@ -1163,21 +1461,3 @@
   Walker->setAutoResetWalker(false);
-
-  // Now optimize the MemoryUse's defining access to point to the nearest
-  // dominating clobbering def.
-  // This ensures that MemoryUse's that are killed by the same store are
-  // immediate users of that store, one of the invariants we guarantee.
-  for (auto DomNode : depth_first(DT)) {
-    BasicBlock *BB = DomNode->getBlock();
-    auto AI = PerBlockAccesses.find(BB);
-    if (AI == PerBlockAccesses.end())
-      continue;
-    AccessList *Accesses = AI->second.get();
-    for (auto &MA : *Accesses) {
-      if (auto *MU = dyn_cast<MemoryUse>(&MA)) {
-        Instruction *Inst = MU->getMemoryInst();
-        MU->setDefiningAccess(Walker->getClobberingMemoryAccess(Inst));
-      }
-    }
-  }
-
+  OptimizeUses(this, Walker, AA, DT).optimizeUses();
   Walker->setAutoResetWalker(true);
----------------
If the renamePass (called just above) gets the right answer, there is no need to OptimizeUses ;-)
Maybe the logic from OptimizeUses can be merged in the renamePass.



Repository:
  rL LLVM

https://reviews.llvm.org/D23032