[PATCH] [SDAG] Make the DAGCombine worklist not grow endlessly due to duplicate insertions.

hfinkel at anl.gov hfinkel at anl.gov
Tue Jul 22 11:07:25 PDT 2014


I just ran the test suite on PPC64/Linux with this patch, and it introduced no failures. ;)

Regarding the PowerPC test changes:

test/CodeGen/PowerPC/complex-return.ll looks like a CodeGen improvement (better store-to-load forwarding).

test/CodeGen/PowerPC/subsumes-pred-regs.ll is also a CodeGen improvement (we changed from comparing the value to 0 to comparing that it is not equal to 1 so that we can reuse the loaded '1' value later).

So I see only improvements here - LGTM.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1082
@@ -1074,1 +1081,3 @@
 
+static bool recursivelyDeleteUnusedNodes(SelectionDAG &DAG, DAGCombiner &DC,
+                                         SDNode *N) {
----------------
It seems like we now have a number of routines that do something like this. SDAG has an:
  void RemoveDeadNode(SDNode *N);

which is also supposed to have behavior close to this (except that it does not updated the DC worklist?). Maybe some refactoring could consolidate things?

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1096
@@ +1095,3 @@
+      for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i)
+        if (N != N->getOperand(i).getNode())
+          Nodes.insert(N->getOperand(i).getNode());
----------------
Tim Northover wrote:
> Is this possible? Isn't that an immediate cycle?
And if it is possible, do you need a isPredecessorOf check instead?

http://reviews.llvm.org/D4616






More information about the llvm-commits mailing list