FranckQC edited a comment on pull request #9482:
URL: https://github.com/apache/tvm/pull/9482#issuecomment-1034443098
Sorry I forgot to answer to the other parts of your message @mbs-octoml .
Many thanks for it by the way!
> Thanks so much for the clear and complete comments, very much appreciate
that.
> Echo @Hzfengsy suggestion to switch to TVMScript for your tests -- perhaps
just pretty printing what you already constructs would be a short cut.
Yes I didn't know about TVMScript before. When writing the tests, I
initially stared at some other test files and got inspiration from them.
Unfortunately the ones I've been looking at might not have been the most
up-to-date way of doing things, sorry for that! :-(
I guess we can still improve the tests later on, but they still accomplish
their main function for now, which is the most important I think, even though
they are probably a little bit more verbose than they would be using TVMScript,
where I could just directly write the expected TIR code instead of unfolding
the TIR code that the CSE pass has produced.
> The code duplication across PrimExpr & Stmnt in
CommonSubexpressionElimintor is unfortunate but suspect removing that would
only obscure things.
Yes, I agree that this duplication is a little bit unfortunate. @masahi did
pointed it out
[here](https://github.com/apache/tvm/pull/9482#discussion_r790201064). I was
also a little bit annoyed with it at the beginning. So I tried to factorize it
out a few times, including an attempt described in my answer
[here](https://github.com/apache/tvm/pull/9482#discussion_r797243979). But all
my attempt ended up with something much too complicated for what we would gain.
In fact, we just happen to want to do almost exactly the same treatment for
an expression and for a statement from an algorithmic point of view, but from a
data-type point of view, quite a things are still different type-wise. That's a
pretty rare situation. In the end, I decided to not force things, and to leave
it like that.
And the positive point of having the VisitStmt() and VisitExpr() not
factorized by some weird magic is that we can still easily customize the
algorithm if at some point we need to, between expressions and statements (I
can't think of any reason we would want that, but still! :) )
Many thanks again!
--
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]
