Mousius commented on code in PR #14483:
URL: https://github.com/apache/tvm/pull/14483#discussion_r1158611432
##########
src/target/parsers/aprofile.cc:
##########
@@ -112,6 +112,7 @@ static TargetFeatures GetFeatures(TargetJSON target) {
bool simd_flag = HasFlag(mcpu, mattr, "+neon") || HasFlag(mcpu, mattr,
"+simd");
bool has_asimd = is_aarch64 || simd_flag;
+ const auto has_sve = HasFlag(mcpu, mattr, "+sve");
Review Comment:
I don't *think* `const` adds any practical value here? The compiler likely
won't do anything different and the values could be overwritten without causing
concern so it's not aiding the programmer.
The repo loosely follows the [Google C++
Guidelines](https://google.github.io/styleguide/cppguide.html#Type_deduction),
which advises against using `auto` unless it adds readability, in this case
`auto` doesn't make it any easier to read or understand. The variable names
could be used to infer what the type might be, but if it's explicit there's no
further effort required on the part of the reader.
Introducing a single variation in the file also introduces a different
weirdism for readers, where this line has been singled out for some unknown
reason and future readers will likely find that at least interesting. Working
in a shared codebase does mean working in a style which is generally consistent
with the existing code so as to avoid such weirdisms.
--
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]
