[PATCH] D35475: [PATCH] [GVN] Ensure replacement instruction dominates its uses

[Pedro Ferreira via Phabricator via llvm-commits]

PFerreira updated this revision to Diff 108080.
PFerreira added a comment.

Keep track of BasicBlocks that have been processed and avoid doing PRE on blocks that haven't been operated on.

This causes the transformation not to be applied in some of the PRE test cases (but fixes the original problem). Not sure if this is desirable.


https://reviews.llvm.org/D35475

Files:
  include/llvm/Transforms/Scalar/GVN.h
  lib/Transforms/Scalar/GVN.cpp


Index: lib/Transforms/Scalar/GVN.cpp
===================================================================
--- lib/Transforms/Scalar/GVN.cpp
+++ lib/Transforms/Scalar/GVN.cpp
@@ -2053,6 +2053,11 @@
       toSplit.push_back(std::make_pair(PREPred->getTerminator(), SuccNum));
       return false;
     }
+
+    // Don't PRE on blocks that have not been numbered.
+    if (NumberedBlocks.count(PREPred) == 0)
+      return false;
+
     // We need to insert somewhere, so let's give it a shot
     PREInstr = CurInst->clone();
     if (!performScalarPREInsertion(PREInstr, PREPred, ValNo)) {
@@ -2103,7 +2108,9 @@
 /// control flow patterns and attempts to perform simple PRE at the join point.
 bool GVN::performPRE(Function &F) {
   bool Changed = false;
+  NumberedBlocks.clear();
   for (BasicBlock *CurrentBlock : depth_first(&F.getEntryBlock())) {
+    NumberedBlocks.insert(CurrentBlock);
     // Nothing to PRE in the entry block.
     if (CurrentBlock == &F.getEntryBlock())
       continue;
@@ -2153,15 +2160,18 @@
 /// Executes one iteration of GVN
 bool GVN::iterateOnFunction(Function &F) {
   cleanupGlobalSets();
+  NumberedBlocks.clear();
 
   // Top-down walk of the dominator tree
   bool Changed = false;
   // Needed for value numbering with phi construction to work.
   // RPOT walks the graph in its constructor and will not be invalidated during
   // processBlock.
   ReversePostOrderTraversal<Function *> RPOT(&F);
-  for (BasicBlock *BB : RPOT)
+  for (BasicBlock *BB : RPOT) {
+    NumberedBlocks.insert(BB);
     Changed |= processBlock(BB);
+  }
 
   return Changed;
 }
Index: include/llvm/Transforms/Scalar/GVN.h
===================================================================
--- include/llvm/Transforms/Scalar/GVN.h
+++ include/llvm/Transforms/Scalar/GVN.h
@@ -111,6 +111,7 @@
   const TargetLibraryInfo *TLI;
   AssumptionCache *AC;
   SetVector<BasicBlock *> DeadBlocks;
+  SmallSet<BasicBlock *, 32> NumberedBlocks;
   OptimizationRemarkEmitter *ORE;
 
   ValueTable VN;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35475.108080.patch
Type: text/x-patch
Size: 2006 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170725/da23ab49/attachment.bin>