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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to