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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1864
+                                          std::vector<unsigned> &SUNumbers) {
+  // Use WorkList to avoid stack overflow on excessively large IRs.
+  struct WorkState {
----------------
reames wrote:
> 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.
It's not for tracking order, it's for preventing us from pushing one node more than once. Imagine the case:

  preds(a) = (x, y, z, b)
  preds(b) = (x, y, z, c)
  preds(c) = (x, y, z, d)


I need the iterator to not push x, y, z into stack 3 times.


https://reviews.llvm.org/D33769