neildhickey commented on code in PR #14483:
URL: https://github.com/apache/tvm/pull/14483#discussion_r1192434201
##########
tests/cpp/target/parsers/aprofile_test.cc:
##########
@@ -290,9 +290,28 @@ TEST(AProfileParser, ArchVersionInvalidLetter) {
ASSERT_EQ(Downcast<Bool>(features.at("has_dotprod")), false);
}
+using AProfileOptionalSVE = testing::TestWithParam<float>;
+TEST_P(AProfileOptionalSVE, OptionalSVESupport) {
+ const auto arch_attr = "+v" + std::to_string(GetParam()) + "a";
+
+ // Check that the "has_sve" feature is not set by default when "+sve" isn't
set as an attribute.
+ auto target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr});
+ auto features = Downcast<TargetFeatures>(target.at("features"));
Review Comment:
I presume the point that @Mousius is making here is that, while auto is fine
to use in the way that you are using it, it doesn't match with the way these
types have been specified in the rest of the file and, therefore, creates a
moment of hesitation when skimming through the code and makes the reader think,
"is something different being done here?". If this change was in a new file it
would be fine, but being in the middle of a file, where in all other cases
these types are specified differently, creates a bit of incongruousness.
--
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]
