zackcquic commented on pull request #7952: URL: https://github.com/apache/tvm/pull/7952#issuecomment-846538241
> > For the reason not to overload superclass, I don't know, but I have a guess, maybe need someone to confirm: > > Currently, only FunctionPass, ModulePass, SequentialPass and PrimFuncPass extends Pass. > > FunctionPass calls InferType at the end (see the comments for detail) while SequentialPass is just like a wrapper. > > In these cases, InterType and SequentialPass maybe not want to be traced/instrumented. > > I disagree with this, mainly because some (perhaps not so) surprising results I got with my initial pass profiling implementation is that `InferType` is called **a lot** and often due to the `FunctionPass`/`SequentialPass` design, leading to quite a bit of compilation slowdown. This information would not have been obtained if we didn't include all passes that are run in the profiling. > > Now, I wonder if we should introduce a "universal" pass context which _every_ PassNode must respect and has access to (perhaps a thread local one)? I definitely want to make sure the timing instrument can time all passes that are run. @altanh Thanks. Just checked, YES, ```InferType``` is called **a lot** than I expected. So I updated like you suggested, instrument in Pass::operator() instead of subclasses' operator(). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
