[PATCH] D58100: [CGP] Replace MaxMemoryUsesToScan cut-off with Use::moveToFrontOfUseList()

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin created this revision.
rtereshin added reviewers: bkramer, chandlerc.
Herald added subscribers: llvm-commits, jdoerfert, jfb.
Herald added a project: LLVM.

This is another take on PR33900: the patch replaces the cut-off based
fix introduced in r308891 with `Use::moveToFrontOfUseList()` calls
that keep memory uses resulting in early exits from `FindAllMemoryUses`
in the beginning of the use list, reducing the overall number of iterations
over the users.

The performance impact is measured on the Function.cpp from
PR33900 compiled with -O3:

| Fix                   | # of iterations | CodeGenPrepare, s | Wall Time, s |
| --------------------- | --------------- | ----------------- | ------------ |
| No (r308891 reverted) | 367,047,160     | 13.28             | 25.17        |
| Before this patch     | 1,521,619       | 0.08              | 11.96        |
| After this patch      | 76,733          | 0.05              | 11.71        |
|

where number of iterations is the number of iterations over the
users in the `FindAllMemoryUses` function only.


Repository:
  rL LLVM

https://reviews.llvm.org/D58100

Files:
  lib/CodeGen/CodeGenPrepare.cpp


Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -4276,10 +4276,6 @@
   return true;
 }
 
-// Max number of memory uses to look at before aborting the search to conserve
-// compile time.
-static constexpr int MaxMemoryUsesToScan = 20;
-
 /// Recursively walk all the uses of I until we find a memory use.
 /// If we find an obviously non-foldable instruction, return true.
 /// Add the ultimately found memory instructions to MemoryUses.
@@ -4287,7 +4283,7 @@
     Instruction *I,
     SmallVectorImpl<std::pair<Instruction *, unsigned>> &MemoryUses,
     SmallPtrSetImpl<Instruction *> &ConsideredInsts, const TargetLowering &TLI,
-    const TargetRegisterInfo &TRI, int SeenInsts = 0) {
+    const TargetRegisterInfo &TRI) {
   // If we already considered this instruction, we're done.
   if (!ConsideredInsts.insert(I).second)
     return false;
@@ -4300,11 +4296,6 @@
 
   // Loop over all the uses, recursively processing them.
   for (Use &U : I->uses()) {
-    // Conservatively return true if we're seeing a large number or a deep chain
-    // of users. This avoids excessive compilation times in pathological cases.
-    if (SeenInsts++ >= MaxMemoryUsesToScan)
-      return true;
-
     Instruction *UserI = cast<Instruction>(U.getUser());
     if (LoadInst *LI = dyn_cast<LoadInst>(UserI)) {
       MemoryUses.push_back(std::make_pair(LI, U.getOperandNo()));
@@ -4313,24 +4304,30 @@
 
     if (StoreInst *SI = dyn_cast<StoreInst>(UserI)) {
       unsigned opNo = U.getOperandNo();
-      if (opNo != StoreInst::getPointerOperandIndex())
+      if (opNo != StoreInst::getPointerOperandIndex()) {
+        U.moveToFrontOfUseList();
         return true; // Storing addr, not into addr.
+      }
       MemoryUses.push_back(std::make_pair(SI, opNo));
       continue;
     }
 
     if (AtomicRMWInst *RMW = dyn_cast<AtomicRMWInst>(UserI)) {
       unsigned opNo = U.getOperandNo();
-      if (opNo != AtomicRMWInst::getPointerOperandIndex())
+      if (opNo != AtomicRMWInst::getPointerOperandIndex()) {
+        U.moveToFrontOfUseList();
         return true; // Storing addr, not into addr.
+      }
       MemoryUses.push_back(std::make_pair(RMW, opNo));
       continue;
     }
 
     if (AtomicCmpXchgInst *CmpX = dyn_cast<AtomicCmpXchgInst>(UserI)) {
       unsigned opNo = U.getOperandNo();
-      if (opNo != AtomicCmpXchgInst::getPointerOperandIndex())
+      if (opNo != AtomicCmpXchgInst::getPointerOperandIndex()) {
+        U.moveToFrontOfUseList();
         return true; // Storing addr, not into addr.
+      }
       MemoryUses.push_back(std::make_pair(CmpX, opNo));
       continue;
     }
@@ -4342,17 +4339,23 @@
         continue;
 
       InlineAsm *IA = dyn_cast<InlineAsm>(CI->getCalledValue());
-      if (!IA) return true;
+      if (!IA) {
+        U.moveToFrontOfUseList();
+        return true;
+      }
 
       // If this is a memory operand, we're cool, otherwise bail out.
-      if (!IsOperandAMemoryOperand(CI, IA, I, TLI, TRI))
+      if (!IsOperandAMemoryOperand(CI, IA, I, TLI, TRI)) {
+        U.moveToFrontOfUseList();
         return true;
+      }
       continue;
     }
 
-    if (FindAllMemoryUses(UserI, MemoryUses, ConsideredInsts, TLI, TRI,
-                          SeenInsts))
+    if (FindAllMemoryUses(UserI, MemoryUses, ConsideredInsts, TLI, TRI)) {
+      U.moveToFrontOfUseList();
       return true;
+    }
   }
 
   return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58100.186397.patch
Type: text/x-patch
Size: 3526 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190212/2886c967/attachment.bin>