[PATCH] D33769: [SelectionDAG] Get rid of recursion in CalcNodeSethiUllmanNumber

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1864
+                                          std::vector<unsigned> &SUNumbers) {
+  // Use WorkList to avoid stack overflow on excessively large IRs.
+  struct WorkState {
----------------
mkazantsev wrote:
> reames wrote:
> > I think you can use a much simpler bit of code here.  The key bit is that we can't compute the current node until all predecessors are done, but that (by assumption from the existing code) the graph is a tree.  (Add an assert that enforces that please!)  Also, Wikipedia helps here.  :)
> > 
> > If you do something along the lines of:
> > while (!Worklist.empty())
> >   Cur = Worklist.pop_back();
> >   if (not all preds done) {
> >      push(Cur)
> >      push(each pred)
> >     continue
> >   }
> >   compute answer using preds
> > }
> > 
> > I think you get the same result right?  This will eliminate the need for the intermediate state.
> We still need a workstate to keep the current pred in order to avoid pushing an element more than once into the stack, but you are right, it can be done in a much more straightforward way. :)
Why do you need the predecessors in order or to track which ones have been previously processed?  

This is a reduction over a set of child nodes.  The order shouldn't matter and the "recursive" walk is responsible for filling in the value.  It shouldn't even matter if the code is DFS or BFS if I'm reading the original right.


https://reviews.llvm.org/D33769