junrushao1994 edited a comment on pull request #8750:
URL: https://github.com/apache/tvm/pull/8750#issuecomment-898820399
Thanks Lily! I agree to have attributes on the `IRModule` level, so that we
could add some metadata and guide the compiler behavior. It is consistent with
the direction we are moving towards as a unified IR.
<del>However, I have some concerns on whether we should remove
`BaseFunc`-level annotations. </del> Sorry I misread!
Another thing I am not so sure about is `DictAttrs` itself. From my PoV it
is basically equivalent to `Map<String, ObjectRef>`, but more ad-hoc in terms
of unnecessary access control and accessor patterns. As an example, recently,
we refactored the ForNode in TIR with the `Map<String, ObjectRef>` as loop
annotation instead of `DictAttrs`. I understand it is not a problem introduced
by this PR, but would love to discuss a little bit if this design is the
direction we want to move forward or move away :-)
BTW, as you mentioned in the description, you might remove `LoweredModule`
in the future. Perhaps I don't have full context here, but I assume it is used
for lowering an IRModule, which is consistent with the direction our compiler
is moving towards ("IRModule => IRModule" transformation). Would you like to
elaborate (if you have time) why we are removing this method? Thanks a lot!
Please let me know what you think :-)
--
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]
