Mousius commented on PR #71: URL: https://github.com/apache/tvm-rfcs/pull/71#issuecomment-1130237913
> I like it overall, though I do have one potential concern: By making it easier to query the architecture compared to cross-architecture features, will developers more often use architecture-specific checks that unnecessarily limit TVM features to specific architectures? Unfortunately, this RFC neither improves nor worsens the situation you describe, I think the only way of really solving how the information is used is to limit the information visible to these functions. `Target.current()` is global and you have access to it all right now (see: https://github.com/apache/tvm/blob/main/python/tvm/topi/arm_cpu/conv2d_int8.py#L174-L177). Ideally people would move to using `.arch.feature` or we only make the `.arch` property visible to the internals (rather than the full `Target`), as you suggested, but that compiler infrastructure change would be out of scope of this RFC. Also, `.arch` may be misleading here, as it's a description of the architecture which means it'll include features such as `.arch.has_dsp`, enabling 2. and 3. based on those available features within the architecture. To further motivate this, the preprocessor can produce a `register_count` property on the `.arch` property if a specific CPU/attr/triple/etc is seen and that could be accessed via `my_target.arch.register_count`. Based on my previous re-review of LLVM, thanks to @tqchen, it might help to use `my_target.features.dsp` rather than `my_target.arch.has_dsp` and clarifying these are `features` available to the `Target`? What do 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]
