[llvm] 2655a70 - [InstCombine] After merging store into successor, queue prev. store to be visited (PR46661)

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2020-07-10T17:49:16+03:00
New Revision: 2655a70a046b67abe3bca01059d8767030f6e1c9

URL: https://github.com/llvm/llvm-project/commit/2655a70a046b67abe3bca01059d8767030f6e1c9
DIFF: https://github.com/llvm/llvm-project/commit/2655a70a046b67abe3bca01059d8767030f6e1c9.diff

LOG: [InstCombine] After merging store into successor, queue prev. store to be visited (PR46661)

We can happen to have a situation with many stores eligible for transform,
but due to our visitation order (top to bottom), when we have processed
the first eligible instruction, we would not try to reprocess the previous
instructions that are now also eligible.

So after we've successfully merged a store that was second-to-last instruction
into successor, if the now-second-to-last instruction is also a such store
that is eligible, add it to worklist to be revisited.

Fixes https://bugs.llvm.org/show_bug.cgi?id=46661

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
    llvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 17f33be374e1..7203850ad24d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -1425,18 +1425,33 @@ Instruction *InstCombiner::visitStoreInst(StoreInst &SI) {
   if (isa<UndefValue>(Val))
     return eraseInstFromFunction(SI);
 
+  auto IsNoopInstrForStoreMerging = [](BasicBlock::iterator BBI) {
+    return isa<DbgInfoIntrinsic>(BBI) ||
+           (isa<BitCastInst>(BBI) && BBI->getType()->isPointerTy());
+  };
+
   // If this store is the second-to-last instruction in the basic block
   // (excluding debug info and bitcasts of pointers) and if the block ends with
   // an unconditional branch, try to move the store to the successor block.
   BBI = SI.getIterator();
   do {
     ++BBI;
-  } while (isa<DbgInfoIntrinsic>(BBI) ||
-           (isa<BitCastInst>(BBI) && BBI->getType()->isPointerTy()));
+  } while (IsNoopInstrForStoreMerging(BBI));
 
   if (BranchInst *BI = dyn_cast<BranchInst>(BBI))
     if (BI->isUnconditional())
-      mergeStoreIntoSuccessor(SI);
+      if (mergeStoreIntoSuccessor(SI)) {
+        // Okay, we've managed to do that. Now, let's see if now-second-to-last
+        // instruction is also a store that we can also sink.
+        BasicBlock::iterator FirstInstr = BBI->getParent()->begin();
+        do {
+          if (BBI != FirstInstr)
+            --BBI;
+        } while (BBI != FirstInstr && IsNoopInstrForStoreMerging(BBI));
+        if (StoreInst *PrevStore = dyn_cast<StoreInst>(BBI))
+          Worklist.add(PrevStore);
+        return nullptr;
+      }
 
   return nullptr;
 }

diff  --git a/llvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll b/llvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll
index f04897b6fb66..5be341fa6228 100644
--- a/llvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll
+++ b/llvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -instcombine -instcombine-infinite-loop-threshold=4 -S | FileCheck %s
+; RUN: opt %s -instcombine -instcombine-infinite-loop-threshold=2 -S | FileCheck %s
 
 @var_7 = external global i8, align 1
 @var_1 = external global i32, align 4