Pass prerequisite handling

I wonder if there isn’t a bug in how prerequisites are handled in SequentialNode::operator() in pass_manager.cc here:

https://github.com/dmlc/tvm/blob/master/src/relay/pass/pass_manager.cc#L417

The way I read this, transitive dependencies are ignored: Suppose pass C depends on pass B which depends on pass A. Then running pass C will cause pass B to be run first. However, pass A is not run because the dependencies of B are not inspected since the dependency tracking is not transitive/recursive.

Though if this is a real bug, what I’m wondering is how this hasn’t caused problems so far?

It will continue invoke the passes in B through the functor if B is sequential. But it will just invoke B but not passes required it B if B is a function/module pass. We probably do not have such as a case in the code/test. I think we may need to add the logic back in the ModulePass and FunctionPass functors to invoke dependent passes in a DFS way. @tqchen

(I kind-of found an example with constant folding, which calls FuseOps, which itself has prerequisites. However, FuseOps is not listed as a prerequisite of constant folding, instead constant folding calls it directly through a new sequence, so the prerequisite does get called, but wouldn’t if it had been declared as a prerequisite. I though this was a surprising use of the API, but later realized that the reason this is being done that way is that FuseOps isn’t being run on all functions, only functions that are selected by constant folding to be relevant for it. Listing it as a prerequisite would run it everywhere, so it makes sense that it’s not, assuming that it shouldn’t run everywhere. So it’s not really a counter-example, but close, and I wonder if there aren’t real counterexamples.)