[GitHub] [tvm-rfcs] Lunderberg commented on pull request #71: Add Target Pre-processing RFC

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

Lunderberg commented on PR #71:
URL: https://github.com/apache/tvm-rfcs/pull/71#issuecomment-1130250850

   > 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?
   
   I like that, and the renaming makes it clear which are boolean parameters and which are variable attributes.  That would also be useful for cleaning up the [Vulkan target](https://github.com/apache/tvm/blob/main/src/target/target_kind.cc#L341), which right now has a large number of boolean attributes that would be better expressed as feature support.
   
   The renaming would also help with avoiding architecture-specific checks at the code review stage.  Since the information defined by the architecture would be pulled over into `my_target.features`, checks that directly access `my_target.arch` should only occur for case (1).


-- 
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