[PATCH] D50990: [InstCombine] When rewriting operands in SimplifyAssociativeOrCommutative, add the original nodes back to the worklist so they'll get eagerly DCEd

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added a reviewer: spatel.

If the original operands become dead they'll stick around until the next iteration of the InstCombine worklist. The worklist builder for that iteration will DCE the last dead node. But if that node was the only thing keeping an earlier node alive, the earlier node will have already been added to the worklist. So earlier nodes won't be DCEd until they are popped from the worklist. Any DCE from the worklist will be counted as a modification that will trigger another iteration of InstCombine even if nothing else was changed.

This patch explicitly adds the nodes back to the worklist so they will be DCEd in the worklist iteration they become dead. The worklist DCE code should take care of adding dead operands to the worklist so the process will remove all dead nodes. This will prevent the worklist builder for the next iteration from having any dead nodes from this. So we can stop another InstCombine iteration from being triggered.

There other places that use setOperand that are probably subject to this same problem. I know we've tried to rewrite operands to save allocations.


https://reviews.llvm.org/D50990

Files:
  lib/Transforms/InstCombine/InstructionCombining.cpp


Index: lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- lib/Transforms/InstCombine/InstructionCombining.cpp
+++ lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -330,6 +330,9 @@
             ClearSubclassDataAfterReassociation(I);
           }
 
+          Worklist.Add(Op0);
+          if (auto *CI = dyn_cast<Instruction>(C))
+            Worklist.Add(CI);
           Changed = true;
           ++NumReassoc;
           continue;
@@ -350,6 +353,9 @@
           // Conservatively clear the optional flags, since they may not be
           // preserved by the reassociation.
           ClearSubclassDataAfterReassociation(I);
+          if (auto *AI = dyn_cast<Instruction>(A))
+            Worklist.Add(AI);
+          Worklist.Add(Op1);
           Changed = true;
           ++NumReassoc;
           continue;
@@ -378,6 +384,9 @@
           // Conservatively clear the optional flags, since they may not be
           // preserved by the reassociation.
           ClearSubclassDataAfterReassociation(I);
+          Worklist.Add(Op0);
+          if (auto *CI = dyn_cast<Instruction>(C))
+            Worklist.Add(CI);
           Changed = true;
           ++NumReassoc;
           continue;
@@ -398,6 +407,9 @@
           // Conservatively clear the optional flags, since they may not be
           // preserved by the reassociation.
           ClearSubclassDataAfterReassociation(I);
+          if (auto *AI = dyn_cast<Instruction>(A))
+            Worklist.Add(AI);
+          Worklist.Add(Op1);
           Changed = true;
           ++NumReassoc;
           continue;
@@ -426,6 +438,8 @@
         // Conservatively clear the optional flags, since they may not be
         // preserved by the reassociation.
         ClearSubclassDataAfterReassociation(I);
+        Worklist.Add(Op0);
+        Worklist.Add(Op1);
 
         Changed = true;
         continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50990.161529.patch
Type: text/x-patch
Size: 1943 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180820/d87997e5/attachment.bin>