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. However, I have some concerns on whether we should remove `BaseFunc`-level annotations. Generally speaking, the attributes we are using on `BaseFunc`-level include: - BaseFunc: https://github.com/apache/tvm/blob/main/include/tvm/ir/function.h#L204 - TIR PrimFunc: https://github.com/apache/tvm/blob/main/include/tvm/tir/function.h#L233 - Relay Function: https://github.com/apache/tvm/blob/main/include/tvm/relay/function.h#L125 Several of the above are actually pretty specific to `BaseFunc`-level, not quite suitable as `IRModule`-level attributes. The particular cases I am thinking of are: - global_symbol: used on TIR's `PrimFunc` to indicate which global symbol this function is compiled to. - noalias: used on TIR's `PrimFunc` to indicate if the input buffers do not alias with each other 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 :-) Please let me know what you think. Also @jroesch @tqchen would you guys share some thoughts on the design? -- 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]
