FranckQC commented on PR #11574: URL: https://github.com/apache/tvm/pull/11574#issuecomment-1149418507
Hopefully, this time everything should be resolved. The last build and all tests from this afternoon were successful, so fingers crossed it will stay the same with the latest commit (I had forgot the minor thing about the curly braces in the previous one from this afternoon, sorry). To summarize, we should now have the best of the two worlds with this particular implementation, since `identify_equiv_terms` has now been brought to the knowledge of the function `SyntacticToSemanticComputations`: - When `identify_equiv_terms` is false (which is the case by default), we go straight from the hashtable to the vector, without doing any necessary work (thank you @tkonolige for the very good point!). - When `identify_equiv_terms `is true (for people ok with much longer compile time but who want to to common-out as much as possible), it will do something better than what it did before this PR, as it now takes benefit of the normal form function (which defines/implies the equivalence relation). Previously, we did not take advantage of that. And by the way, now this "identify_equiv_terms = true" mode really is usable for someone who wishes to (it wasn't fully plugged before). Despite all of that, this pass still won't be cheap at compile time, for sure (especially for programs with a lot of things to common out in cascade). But I think it should be acceptable. When I wrote the pass, I focused more on trying to not miss opportunities for commonings, and on the correctness on the pass, rather that on making the pass as cheap as possible at compile time. I guess it's often a tradeoff. I hope that's ok for most users. Many thanks for having helped to improve the pass everyone, I appreciate it! Franck -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
