[GitHub] [tvm-rfcs] Mousius commented on pull request #29: Migrating Target Attributes to IRModule

[GitBox <gi...@apache.org>]

Mousius commented on pull request #29:
URL: https://github.com/apache/tvm-rfcs/pull/29#issuecomment-932143366


   Hi @electriclilies,
   
   Following up on your questions :smile_cat: Overall I think we're thinking the same things in terms of direction of travel!
   
   > Overall this seems like a good change! One question I have is what the relationship between PassContext and the IRModule attributes will be in the future. I think that ideally we'd have all the information about transformations that will be done to the IRModule in one place.
   
   I completely agree, I think the direction of travel should be towards removing it or integrating it with the `IRModule` so you have a representation of the compilation at any given time.
   
   > Right now, the PassContext has some module-level configuration options, but it also has options that are extracted from the PassContext within passes and modifies the behavior of those passes. I think that we should at least move the module-level configuration options into the IRModule attributes, but I'm not sure about what we should do with the other options.
   
   This would be an RFC of interest to me, moving them into attributes and only passing the IRModule seems sensible - the `Pass` level configuration we should probably check to see if they could be more generic options or whether we can recompute the transforms efficiently enough they don't need to directly mutate the `IRModule`?
   
   > Another question is whether we want to distinguish between attributes / config options that are pass-invariant and attributes that passes can change. This would make it easier to make sure that the pass-invariant ones are being propagated properly. (We don't need to resolve these questions before the PR goes in, just mentioning them while I'm thinking about it!)
   
   Agree, we should probably try to freeze attributes which aren't meant to change (hopefully most of them) and that would hopefully make things simpler for everyone. Unsure if TVM has support for this in `attrs` right now?


-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org