From c6587cdcd946a0e0595ba2bccc14e1acd82737b7 Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Mon, 15 Jan 2024 15:10:37 +0000 Subject: [PATCH 1/9] [Target] Use LLVM target parser for determining Arm(R) A-Profile Architecture features Currently, target features are determined by a set of fixed checks on the target string. This works well for checking support of a small number of simple features, but it doesn't scale. Some problems include: - There are many non-trivial conditions for which a feature may(not) be available. It is easy to miss these with the current implementation. - The inclusion of some features in a target string can imply other features. For example, "+sve" implies "+neon". This currently isn't taken into account. - The tests in tests/cpp/target/parsers/aprofile_test.c suggest that targets such as "llvm -mcpu=cortex-a+neon" and "llvm -mattr=+noneon" are supported target strings. The features will be correctly parsed in TVM, however, they are not valid in LLVM. Therefore, it's possible that TVM and LLVM have different understanding of the features available. This commit uses the more robust LLVM target parser to determine support for the features in TVM. It leverages previous infrastructure added to TVM for obtaining a list of all supported features given an input target, and uses this to check the existance of certain features we're interested in. It should be trivial to grow this list over time. As a result of this change, the problems mentioned above are solved. In the current form, this commit drops support for target strings such as "llvm -mcpu=cortex-a+neon" and "llvm -mattr=+noneon". A scan of the codebase suggests this functionality is not in use (only in test cases). Should we feel the need to support them, or have a smoother migration for downstream users of TVM we can add a translator to the parser to convert these into LLVM compatible targets. Change-Id: Ic2bf3b68c8af74025ec388d304bd014624c0c585 --- src/target/llvm/llvm_instance.cc | 37 ++-- src/target/llvm/llvm_instance.h | 13 +- src/target/llvm/llvm_module.cc | 7 +- src/target/parsers/aprofile.cc | 72 ++------ tests/cpp/target/parsers/aprofile_test.cc | 166 ++++++++++-------- .../strategy/test_select_implementation.py | 2 +- 6 files changed, 148 insertions(+), 149 deletions(-) diff --git a/src/target/llvm/llvm_instance.cc b/src/target/llvm/llvm_instance.cc index a1359b7850a4..765d7a5cf98b 100644 --- a/src/target/llvm/llvm_instance.cc +++ b/src/target/llvm/llvm_instance.cc @@ -199,21 +199,23 @@ std::ostream& operator<<(std::ostream& os, const LLVMTargetInfo::Option& opt) { return os; } -LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const Target& target) { - triple_ = target->GetAttr("mtriple").value_or("default"); +LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const Target& target) + : LLVMTargetInfo(instance, target->Export()) {} +LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const TargetJSON& target) { + triple_ = Downcast(target.Get("mtriple").value_or(String("default"))); if (triple_.empty() || triple_ == "default") { triple_ = llvm::sys::getDefaultTargetTriple(); } - cpu_ = target->GetAttr("mcpu").value_or(defaults::cpu); + cpu_ = Downcast(target.Get("mcpu").value_or(String(defaults::cpu))); - if (const Optional>& v = target->GetAttr>("mattr")) { + if (const auto& v = Downcast>>(target.Get("mattr"))) { for (const String& s : v.value()) { attrs_.push_back(s); } } // llvm module target - if (target->kind->name == "llvm") { + if (Downcast(target.Get("kind")) == "llvm") { // legalize -mcpu with the target -mtriple auto arches = GetAllLLVMTargetArches(); bool has_arch = @@ -224,7 +226,7 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const Target& target) { } } - if (const Optional>& v = target->GetAttr>("cl-opt")) { + if (const auto& v = Downcast>>(target.Get("cl-opt"))) { llvm::StringMap& options = llvm::cl::getRegisteredOptions(); bool parse_error = false; for (const String& s : v.value()) { @@ -245,7 +247,7 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const Target& target) { } llvm::FloatABI::ABIType float_abi = llvm::FloatABI::Default; - if (const Optional& v = target->GetAttr("mfloat-abi")) { + if (const auto& v = Downcast>(target.Get("mfloat-abi"))) { String value = v.value(); if (value == "hard") { float_abi = llvm::FloatABI::Hard; @@ -283,14 +285,14 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const Target& target) { target_options_.NoInfsFPMath = false; target_options_.NoNaNsFPMath = true; target_options_.FloatABIType = float_abi; - if (const Optional& v = target->GetAttr("mabi")) { - target_options_.MCOptions.ABIName = v.value(); + if (target.find("mabi") != target.end()) { + target_options_.MCOptions.ABIName = Downcast(target.Get("mabi")); } - auto maybe_level = target->GetAttr("opt-level"); + auto maybe_level = Downcast(target.Get("opt-level")); #if TVM_LLVM_VERSION <= 170 if (maybe_level.defined()) { - int level = maybe_level.value()->value; + int level = maybe_level->value; if (level <= 0) { opt_level_ = llvm::CodeGenOpt::None; } else if (level == 1) { @@ -327,7 +329,7 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const Target& target) { // Fast math options auto GetBoolFlag = [&target](llvm::StringRef flag) -> bool { - return target->GetAttr(flag.str()).value_or(Bool(false)); + return Downcast(target.Get(flag.str()).value_or(Bool(false))); }; if (GetBoolFlag("fast-math")) { #if TVM_LLVM_VERSION >= 60 @@ -850,7 +852,7 @@ const Array LLVMTargetInfo::GetAllLLVMTargetArches() const { return cpu_arches; } -const Array LLVMTargetInfo::GetAllLLVMCpuFeatures() const { +const Map LLVMTargetInfo::GetAllLLVMCpuFeatures() const { std::string feats = ""; for (const auto& attr : attrs_) { feats += feats.empty() ? attr : ("," + attr); @@ -864,10 +866,11 @@ const Array LLVMTargetInfo::GetAllLLVMCpuFeatures() const { #else MCInfo->getAllProcessorFeatures(); #endif - Array cpu_features; + // TVM doesn't have an FFI friendly Set, so use a Map instead for now + Map cpu_features; for (const auto& feat : llvm_features) { if (MCInfo->checkFeatures("+" + std::string(feat.Key))) { - cpu_features.push_back(feat.Key); + cpu_features.Set(feat.Key, ""); } } @@ -877,9 +880,7 @@ const Array LLVMTargetInfo::GetAllLLVMCpuFeatures() const { const bool LLVMTargetInfo::TargetHasCPUFeature(const std::string& feature) const { // lookup features for `-mcpu` auto feats = GetAllLLVMCpuFeatures(); - bool has_feature = - std::any_of(feats.begin(), feats.end(), [&](const auto& var) { return var == feature; }); - + bool has_feature = feats.find(feature) != feats.end(); return has_feature; } diff --git a/src/target/llvm/llvm_instance.h b/src/target/llvm/llvm_instance.h index f3948b7a01d2..fd63140a0b37 100644 --- a/src/target/llvm/llvm_instance.h +++ b/src/target/llvm/llvm_instance.h @@ -156,6 +156,14 @@ class LLVMTargetInfo { */ // NOLINTNEXTLINE(runtime/references) LLVMTargetInfo(LLVMInstance& scope, const std::string& target_str); + /*! + * \brief Constructs LLVMTargetInfo from `Target` + * \param scope LLVMInstance object + * \param target TVM JSON Target object for target "llvm" + */ + // NOLINTNEXTLINE(runtime/references) + LLVMTargetInfo(LLVMInstance& instance, const TargetJSON& target); + /*! * \brief Destroys LLVMTargetInfo object */ @@ -290,11 +298,12 @@ class LLVMTargetInfo { /*! * \brief Get all CPU features from target - * \return list with all valid cpu features + * \return Map with all valid cpu features as keys and empty string as value. The Map + * is intended to be used as a Set, which TVM does not currently support. * \note The features are fetched from the LLVM backend using the target `-mtriple` * and the `-mcpu` architecture, but also consider the `-mattr` attributes. */ - const Array GetAllLLVMCpuFeatures() const; + const Map GetAllLLVMCpuFeatures() const; /*! * \brief Check the target if has a specific cpu feature diff --git a/src/target/llvm/llvm_module.cc b/src/target/llvm/llvm_module.cc index c332314a3e6c..baa68feedfa2 100644 --- a/src/target/llvm/llvm_module.cc +++ b/src/target/llvm/llvm_module.cc @@ -697,12 +697,12 @@ TVM_REGISTER_GLOBAL("target.llvm_get_cpu_archlist") }); TVM_REGISTER_GLOBAL("target.llvm_get_cpu_features") - .set_body_typed([](const Target& target) -> Array { + .set_body_typed([](const Target& target) -> Map { auto use_target = target.defined() ? target : Target::Current(false); // ignore non "llvm" target if (target.defined()) { if (target->kind->name != "llvm") { - return Array{}; + return {}; } } auto llvm_instance = std::make_unique(); @@ -722,8 +722,7 @@ TVM_REGISTER_GLOBAL("target.llvm_cpu_has_feature") auto llvm_instance = std::make_unique(); LLVMTargetInfo llvm_backend(*llvm_instance, use_target); auto cpu_features = llvm_backend.GetAllLLVMCpuFeatures(); - bool has_feature = std::any_of(cpu_features.begin(), cpu_features.end(), - [&](auto& var) { return var == feature; }); + bool has_feature = cpu_features.find(feature) != cpu_features.end(); return has_feature; }); diff --git a/src/target/parsers/aprofile.cc b/src/target/parsers/aprofile.cc index 622ec5cc3fbf..cc1dc4852388 100644 --- a/src/target/parsers/aprofile.cc +++ b/src/target/parsers/aprofile.cc @@ -24,9 +24,11 @@ #include "aprofile.h" +#include #include #include "../../support/utils.h" +#include "../llvm/llvm_instance.h" namespace tvm { namespace target { @@ -52,33 +54,6 @@ double GetArchVersion(Optional> attr) { return GetArchVersion(attr.value()); } -static inline bool HasFlag(String attr, std::string flag) { - std::string attr_str = attr; - return attr_str.find(flag) != std::string::npos; -} - -static inline bool HasFlag(Optional attr, std::string flag) { - if (!attr) { - return false; - } - return HasFlag(attr.value(), flag); -} - -static inline bool HasFlag(Optional> attr, std::string flag) { - if (!attr) { - return false; - } - Array attr_array = attr.value(); - - auto matching_attr = std::find_if(attr_array.begin(), attr_array.end(), - [flag](String attr_str) { return HasFlag(attr_str, flag); }); - return matching_attr != attr_array.end(); -} - -static bool HasFlag(Optional mcpu, Optional> mattr, std::string flag) { - return HasFlag(mcpu, flag) || HasFlag(mattr, flag); -} - bool IsAArch32(Optional mtriple, Optional mcpu) { if (mtriple) { bool is_mprofile = mcpu && support::StartsWith(mcpu.value(), "cortex-m"); @@ -102,38 +77,25 @@ bool IsArch(TargetJSON attrs) { } static TargetFeatures GetFeatures(TargetJSON target) { - Optional mcpu = Downcast>(target.Get("mcpu")); - Optional mtriple = Downcast>(target.Get("mtriple")); - Optional> mattr = Downcast>>(target.Get("mattr")); - - const double arch_version = GetArchVersion(mattr); - - const bool is_aarch64 = IsAArch64(mtriple); + String kind = Downcast(target.Get("kind")); + ICHECK_EQ(kind, "llvm") << "Expected target kind 'llvm', but got '" << kind << "'"; - const bool simd_flag = HasFlag(mcpu, mattr, "+neon") || HasFlag(mcpu, mattr, "+simd"); - const bool has_asimd = is_aarch64 || simd_flag; - const bool has_sve = HasFlag(mcpu, mattr, "+sve"); - - const bool i8mm_flag = HasFlag(mcpu, mattr, "+i8mm"); - const bool i8mm_disable = HasFlag(mcpu, mattr, "+noi8mm"); - const bool i8mm_default = arch_version >= 8.6; - const bool i8mm_support = arch_version >= 8.2 && arch_version <= 8.5; - const bool has_i8mm = (i8mm_default && !i8mm_disable) || (i8mm_support && i8mm_flag); + Optional mtriple = Downcast>(target.Get("mtriple")); - const bool dotprod_flag = HasFlag(mcpu, mattr, "+dotprod"); - const bool dotprod_disable = HasFlag(mcpu, mattr, "+nodotprod"); - const bool dotprod_default = arch_version >= 8.4; - const bool dotprod_support = arch_version >= 8.2 && arch_version <= 8.3; - const bool has_dotprod = - (dotprod_default && !dotprod_disable) || (dotprod_support && dotprod_flag); + auto llvm_instance = std::make_unique(); + codegen::LLVMTargetInfo llvm_target(*llvm_instance, target); + Map features = llvm_target.GetAllLLVMCpuFeatures(); - const bool fp16_flag = HasFlag(mcpu, mattr, "+fullfp16"); - const bool fp16_support = arch_version >= 8.2; - const bool has_fp16_simd = fp16_support && (fp16_flag || has_sve); + auto has_feature = [features](const String& feature) { + return features.find(feature) != features.end(); + }; - return {{"is_aarch64", Bool(is_aarch64)}, {"has_asimd", Bool(has_asimd)}, - {"has_sve", Bool(has_sve)}, {"has_dotprod", Bool(has_dotprod)}, - {"has_matmul_i8", Bool(has_i8mm)}, {"has_fp16_simd", Bool(has_fp16_simd)}}; + return {{"is_aarch64", Bool(IsAArch64(mtriple))}, + {"has_asimd", Bool(has_feature("neon"))}, + {"has_sve", Bool(has_feature("sve"))}, + {"has_dotprod", Bool(has_feature("dotprod"))}, + {"has_matmul_i8", Bool(has_feature("i8mm"))}, + {"has_fp16_simd", Bool(has_feature("fullfp16"))}}; } static Array MergeKeys(Optional> existing_keys) { diff --git a/tests/cpp/target/parsers/aprofile_test.cc b/tests/cpp/target/parsers/aprofile_test.cc index fa85d1c32989..4c08049d21c9 100644 --- a/tests/cpp/target/parsers/aprofile_test.cc +++ b/tests/cpp/target/parsers/aprofile_test.cc @@ -19,6 +19,7 @@ #include "../src/target/parsers/aprofile.h" +#include #include #include @@ -29,6 +30,8 @@ namespace target { namespace parsers { namespace aprofile { +using ::testing::HasSubstr; + static float defaultI8MM = 8.6; static float optionalI8MM[] = {8.2, 8.3, 8.4, 8.5}; static float defaultDotProd = 8.4; @@ -38,15 +41,25 @@ class AProfileOptionalI8MM : public testing::TestWithParam {}; class AProfileOptionalDotProd : public testing::TestWithParam {}; static TargetFeatures ParseTargetWithAttrs(String mcpu, String mtriple, Array mattr) { - return ParseTarget({ - {"mcpu", mcpu}, + TargetJSON target_json = { + {"kind", String("llvm")}, {"mtriple", mtriple}, {"mattr", mattr}, - }); + }; + if (mcpu != "") { + target_json.Set("mcpu", mcpu); + } + return ParseTarget(target_json); +} + +std::string FloatToStringWithoutTrailingZeros(float value) { + std::stringstream ss; + ss << value; + return ss.str(); } TEST(AProfileParser, ParseTargetKeys) { - TargetJSON target = ParseTarget({}); + TargetJSON target = ParseTarget({{"kind", String("llvm")}}); Array keys = Downcast>(target.at("keys")); ASSERT_EQ(keys.size(), 2); ASSERT_EQ(keys[0], "arm_cpu"); @@ -55,6 +68,7 @@ TEST(AProfileParser, ParseTargetKeys) { TEST(AProfileParser, ParseTargetWithExistingKeys) { TargetJSON target = ParseTarget({ + {"kind", String("llvm")}, {"keys", Array{"cpu"}}, }); TargetFeatures features = Downcast(target.at("features")); @@ -66,6 +80,7 @@ TEST(AProfileParser, ParseTargetWithExistingKeys) { TEST(AProfileParser, ParseTargetWithDuplicateKey) { TargetJSON target = ParseTarget({ + {"kind", String("llvm")}, {"keys", Array{"cpu", "arm_cpu"}}, }); TargetFeatures features = Downcast(target.at("features")); @@ -76,13 +91,10 @@ TEST(AProfileParser, ParseTargetWithDuplicateKey) { } TEST(AProfileParser, ParseTargetDefaults) { - TargetJSON target = ParseTarget({}); + TargetJSON target = ParseTarget({{"kind", String("llvm")}}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(Downcast(features.at("is_aarch64")), false); - ASSERT_EQ(Downcast(features.at("has_asimd")), false); - ASSERT_EQ(Downcast(features.at("has_dotprod")), false); - ASSERT_EQ(Downcast(features.at("has_matmul_i8")), false); } TEST(AProfileParser, IsAArch64Triple) { @@ -111,6 +123,7 @@ TEST(AProfileParser, IsAArch32Triple) { TEST(AProfileParser, IsAArch32BlankCPU) { TargetJSON target = ParseTarget({ + {"kind", String("llvm")}, {"mtriple", String("arm-unknown-linux-gnu")}, }); TargetFeatures features = Downcast(target.at("features")); @@ -155,11 +168,12 @@ TEST(AProfileParser, AArch64HasASIMD) { ASSERT_EQ(Downcast(features.at("has_asimd")), true); } -TEST(AProfileParser, AArch32NoASIMD) { +TEST(AProfileParser, AArch32ASIMD) { TargetJSON target = ParseTargetWithAttrs("", "armv8a-arm-none-eabi", {}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_asimd")), false); + // TODO(lhutton1) LLVM reports that Neon is support with this target string. Is that true? + ASSERT_EQ(Downcast(features.at("has_asimd")), true); } TEST(AProfileParser, AArch32HasASIMDWithOption) { @@ -167,11 +181,6 @@ TEST(AProfileParser, AArch32HasASIMDWithOption) { TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_asimd")), true); - - target = ParseTargetWithAttrs("cortex-a+simd", "armv8a-arm-none-eabi", {""}); - features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_asimd")), true); } TEST(AProfileParser, AArch32HasASIMDWithAlternativeOption) { @@ -179,23 +188,10 @@ TEST(AProfileParser, AArch32HasASIMDWithAlternativeOption) { TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_asimd")), true); - - target = ParseTargetWithAttrs("cortex-a+neon", "armv8a-arm-none-eabi", {""}); - features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_asimd")), true); -} - -TEST(AProfileParser, NoI8MMSupport) { - std::string attr = "+v8.0a"; - TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {attr, "+i8mm"}); - TargetFeatures features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_matmul_i8")), false); } TEST(AProfileParser, DefaultI8MMSupport) { - std::string arch_attr = "+v" + std::to_string(defaultI8MM) + "a"; + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultI8MM) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); @@ -203,20 +199,15 @@ TEST(AProfileParser, DefaultI8MMSupport) { } TEST(AProfileParser, DefaultI8MMSupportDisable) { - std::string arch_attr = "+v" + std::to_string(defaultI8MM) + "a"; - TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "+noi8mm"}); + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultI8MM) + "a"; + TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "-i8mm"}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_matmul_i8")), false); - - target = ParseTargetWithAttrs("cortex-a+noi8mm", "aarch64-arm-none-eabi", {arch_attr}); - features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_matmul_i8")), false); } TEST_P(AProfileOptionalI8MM, OptionalI8MMSupport) { - std::string arch_attr = "+v" + std::to_string(GetParam()) + "a"; + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); @@ -227,23 +218,10 @@ TEST_P(AProfileOptionalI8MM, OptionalI8MMSupport) { features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_matmul_i8")), true); - - target = ParseTargetWithAttrs("cortex-a+i8mm", "aarch64-arm-none-eabi", {arch_attr}); - features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_matmul_i8")), true); -} - -TEST(AProfileParser, NoDotProdSupport) { - std::string attr = "+v8.0a"; - TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {attr, "+dotprod"}); - TargetFeatures features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_dotprod")), false); } TEST(AProfileParser, DefaultDotProdSupport) { - std::string arch_attr = "+v" + std::to_string(defaultDotProd) + "a"; + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultDotProd) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); @@ -251,20 +229,15 @@ TEST(AProfileParser, DefaultDotProdSupport) { } TEST(AProfileParser, DefaultDotProdSupportDisable) { - std::string arch_attr = "+v" + std::to_string(defaultDotProd) + "a"; - TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "+nodotprod"}); + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultDotProd) + "a"; + TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "-dotprod"}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_dotprod")), false); - - target = ParseTargetWithAttrs("cortex-a+nodotprod", "aarch64-arm-none-eabi", {arch_attr}); - features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_dotprod")), false); } TEST_P(AProfileOptionalDotProd, OptionalDotProdSupport) { - std::string arch_attr = "+v" + std::to_string(GetParam()) + "a"; + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); @@ -275,15 +248,10 @@ TEST_P(AProfileOptionalDotProd, OptionalDotProdSupport) { features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_dotprod")), true); - - target = ParseTargetWithAttrs("cortex-a+dotprod", "aarch64-arm-none-eabi", {arch_attr}); - features = Downcast(target.at("features")); - ASSERT_EQ(IsArch(target), true); - ASSERT_EQ(Downcast(features.at("has_dotprod")), true); } TEST(AProfileParser, ArchVersionInvalidLetter) { - std::string arch_attr = "+v" + std::to_string(defaultDotProd) + "b"; + std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultDotProd) + "b"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); @@ -292,7 +260,7 @@ TEST(AProfileParser, ArchVersionInvalidLetter) { using AProfileOptionalSVE = testing::TestWithParam; TEST_P(AProfileOptionalSVE, OptionalSVESupport) { - const std::string arch_attr = "+v" + std::to_string(GetParam()) + "a"; + const std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; // Check that the "has_sve" feature is not set by default when "+sve" isn't set as an attribute. TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); @@ -307,9 +275,25 @@ TEST_P(AProfileOptionalSVE, OptionalSVESupport) { EXPECT_TRUE(Downcast(features.at("has_sve"))); } +TEST(AProfileDefaultSVE, DefaultSVESupportSVESupport) { + const std::string arch_attr = "+v9a"; + + // Check that the "has_sve" feature is not set by default when "+sve" isn't set as an attribute. + TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); + TargetFeatures features = Downcast(target.at("features")); + EXPECT_TRUE(IsArch(target)); + EXPECT_TRUE(Downcast(features.at("has_sve"))); + + // Check that the "has_sve" feature is set when "+sve" is explicitly set as an attribute. + target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "+sve"}); + features = Downcast(target.at("features")); + EXPECT_TRUE(IsArch(target)); + EXPECT_TRUE(Downcast(features.at("has_sve"))); +} + using AProfileOptionalFP16 = testing::TestWithParam; TEST_P(AProfileOptionalFP16, OptionalFP16Support) { - const std::string arch_attr = "+v" + std::to_string(GetParam()) + "a"; + const std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; // Check that the "has_fp16_simd" feature is not set by default when "+fullfp16" isn't set as an // attribute. @@ -332,13 +316,57 @@ TEST_P(AProfileOptionalFP16, OptionalFP16Support) { EXPECT_TRUE(Downcast(features.at("has_fp16_simd"))); } +TEST(AProfileDefaultFP16, DefaultFP16Support) { + const std::string arch_attr = "+v9a"; + + // Check that the "has_fp16_simd" feature is not set by default when "+fullfp16" isn't set as an + // attribute. + TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); + TargetFeatures features = Downcast(target.at("features")); + EXPECT_TRUE(IsArch(target)); + EXPECT_TRUE(Downcast(features.at("has_fp16_simd"))); + + // Check that the "has_fp16_simd" feature is set when "+fullfp16" is explicitly set as an + // attribute. + target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "+fullfp16"}); + features = Downcast(target.at("features")); + EXPECT_TRUE(IsArch(target)); + EXPECT_TRUE(Downcast(features.at("has_fp16_simd"))); + + // Check that the "has_fp16_simd" feature is set when "+sve" is explicitly set as an attribute. + target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "+sve"}); + features = Downcast(target.at("features")); + EXPECT_TRUE(IsArch(target)); + EXPECT_TRUE(Downcast(features.at("has_fp16_simd"))); +} + +TEST(AProfileParser, ImpliedFeature) { + TargetJSON target = ParseTargetWithAttrs("", "aarch64-linux-gnu", {"+sve2"}); + TargetFeatures features = Downcast(target.at("features")); + EXPECT_TRUE(Downcast(features.at("has_sve"))); + EXPECT_TRUE(Downcast(features.at("has_asimd"))); +} + +TEST(AProfileParser, UnexpectedTargetKind) { + EXPECT_THROW( + { + try { + ParseTarget({{"kind", String("c")}}); + } catch (const tvm::InternalError& e) { + EXPECT_THAT(e.what(), HasSubstr("Expected target kind 'llvm', but got 'c'")); + throw; + } + }, + tvm::InternalError); +} + INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalI8MM, ::testing::ValuesIn(optionalI8MM)); INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalDotProd, ::testing::ValuesIn(optionalDotProd)); INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalSVE, - ::testing::Values(8.0, 8.1, 8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9, 9.0)); + ::testing::Values(8.0, 8.1, 8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9)); INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalFP16, - ::testing::Values(8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9, 9.0)); + ::testing::Values(8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9)); } // namespace aprofile } // namespace parsers diff --git a/tests/python/relay/strategy/test_select_implementation.py b/tests/python/relay/strategy/test_select_implementation.py index 0ab00e550895..038c53ee65a1 100644 --- a/tests/python/relay/strategy/test_select_implementation.py +++ b/tests/python/relay/strategy/test_select_implementation.py @@ -119,7 +119,7 @@ def _get_conv2d_impl(dtype, target): ), ( "llvm --device=arm_cpu --mtriple=aarch64-linux-gnu -mattr=+v9a", - "conv2d_NHWC_quantized_interleaved_without_transform.arm_cpu", + "conv2d_NHWC_quantized_native_without_transform.arm_cpu", ), ], ) From 392f796a7dc718848ab0167e139efc79fbe38ed8 Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Sat, 20 Jan 2024 14:00:08 +0000 Subject: [PATCH 2/9] Fix ci build and testing `ci-gpu` - Made 'codegen' namespacing more specific `ci-i386` - Required a more modern version of LLVM for the aprofile tests `ci-hexagon` - Skipped the aprofile tests if LLVM had not been built with the correct targets Change-Id: I792b5994fcea52c74b40e040630db1bbd96ca16c --- src/target/parsers/aprofile.cc | 4 +- tests/cpp/target/parsers/aprofile_test.cc | 105 +++++++++++++++------- 2 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/target/parsers/aprofile.cc b/src/target/parsers/aprofile.cc index cc1dc4852388..524688a1d68a 100644 --- a/src/target/parsers/aprofile.cc +++ b/src/target/parsers/aprofile.cc @@ -82,8 +82,8 @@ static TargetFeatures GetFeatures(TargetJSON target) { Optional mtriple = Downcast>(target.Get("mtriple")); - auto llvm_instance = std::make_unique(); - codegen::LLVMTargetInfo llvm_target(*llvm_instance, target); + auto llvm_instance = std::make_unique(); + tvm::codegen::LLVMTargetInfo llvm_target(*llvm_instance, target); Map features = llvm_target.GetAllLLVMCpuFeatures(); auto has_feature = [features](const String& feature) { diff --git a/tests/cpp/target/parsers/aprofile_test.cc b/tests/cpp/target/parsers/aprofile_test.cc index 4c08049d21c9..d7dc063b320b 100644 --- a/tests/cpp/target/parsers/aprofile_test.cc +++ b/tests/cpp/target/parsers/aprofile_test.cc @@ -17,6 +17,8 @@ * under the License. */ +#if TVM_LLVM_VERSION > 120 + #include "../src/target/parsers/aprofile.h" #include @@ -25,6 +27,8 @@ #include #include +#include "../src/target/llvm/llvm_instance.h" + namespace tvm { namespace target { namespace parsers { @@ -37,8 +41,38 @@ static float optionalI8MM[] = {8.2, 8.3, 8.4, 8.5}; static float defaultDotProd = 8.4; static float optionalDotProd[] = {8.2, 8.3}; -class AProfileOptionalI8MM : public testing::TestWithParam {}; -class AProfileOptionalDotProd : public testing::TestWithParam {}; +class AProfileParser : public ::testing::Test { + public: + AProfileParser() { + auto llvm_instance = std::make_unique(); + codegen::LLVMTargetInfo llvm_backend(*llvm_instance, "llvm"); + Array targets = llvm_backend.GetAllLLVMTargets(); + int expected_target_count = 0; + for (String target : targets) { + if (target == "aarch64" || target == "arm") { + expected_target_count += 1; + } + } + if (expected_target_count >= 2) { + has_aarch64_and_arm_targets = true; + } + } + + // Check that LLVM has been compiled with the required targets, otherwise skip the test. + // Unfortunately, googletest doesn't let you call GTEST_SKIP in SetUpTestSuite() to skip + // the whole suite of tests, so a cached result is checked before each test is run instead. + void SetUp() override { + if (!has_aarch64_and_arm_targets) { + GTEST_SKIP() << "Skipping as LLVM has not been built for Arm(R)-based targets."; + } + } + + private: + bool has_aarch64_and_arm_targets = false; +}; + +class AProfileParserTestWithParam : public AProfileParser, + public testing::WithParamInterface {}; static TargetFeatures ParseTargetWithAttrs(String mcpu, String mtriple, Array mattr) { TargetJSON target_json = { @@ -58,7 +92,7 @@ std::string FloatToStringWithoutTrailingZeros(float value) { return ss.str(); } -TEST(AProfileParser, ParseTargetKeys) { +TEST_F(AProfileParser, ParseTargetKeys) { TargetJSON target = ParseTarget({{"kind", String("llvm")}}); Array keys = Downcast>(target.at("keys")); ASSERT_EQ(keys.size(), 2); @@ -66,7 +100,7 @@ TEST(AProfileParser, ParseTargetKeys) { ASSERT_EQ(keys[1], "cpu"); } -TEST(AProfileParser, ParseTargetWithExistingKeys) { +TEST_F(AProfileParser, ParseTargetWithExistingKeys) { TargetJSON target = ParseTarget({ {"kind", String("llvm")}, {"keys", Array{"cpu"}}, @@ -78,7 +112,7 @@ TEST(AProfileParser, ParseTargetWithExistingKeys) { ASSERT_EQ(keys[1], "arm_cpu"); } -TEST(AProfileParser, ParseTargetWithDuplicateKey) { +TEST_F(AProfileParser, ParseTargetWithDuplicateKey) { TargetJSON target = ParseTarget({ {"kind", String("llvm")}, {"keys", Array{"cpu", "arm_cpu"}}, @@ -90,21 +124,21 @@ TEST(AProfileParser, ParseTargetWithDuplicateKey) { ASSERT_EQ(keys[1], "arm_cpu"); } -TEST(AProfileParser, ParseTargetDefaults) { +TEST_F(AProfileParser, ParseTargetDefaults) { TargetJSON target = ParseTarget({{"kind", String("llvm")}}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(Downcast(features.at("is_aarch64")), false); } -TEST(AProfileParser, IsAArch64Triple) { +TEST_F(AProfileParser, IsAArch64Triple) { TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {""}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("is_aarch64")), true); } -TEST(AProfileParser, IsAArch32Triple) { +TEST_F(AProfileParser, IsAArch32Triple) { TargetJSON target = ParseTargetWithAttrs("", "armv7a-arm-none-eabi", {""}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); @@ -121,7 +155,7 @@ TEST(AProfileParser, IsAArch32Triple) { ASSERT_EQ(Downcast(features.at("is_aarch64")), false); } -TEST(AProfileParser, IsAArch32BlankCPU) { +TEST_F(AProfileParser, IsAArch32BlankCPU) { TargetJSON target = ParseTarget({ {"kind", String("llvm")}, {"mtriple", String("arm-unknown-linux-gnu")}, @@ -130,7 +164,7 @@ TEST(AProfileParser, IsAArch32BlankCPU) { ASSERT_EQ(IsArch(target), true); } -TEST(AProfileParser, IsAArch32TripleWithAProfile) { +TEST_F(AProfileParser, IsAArch32TripleWithAProfile) { TargetJSON target = ParseTargetWithAttrs("cortex-a53", "armv7a-arm-none-eabi", {""}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); @@ -147,7 +181,7 @@ TEST(AProfileParser, IsAArch32TripleWithAProfile) { ASSERT_EQ(Downcast(features.at("is_aarch64")), false); } -TEST(AProfileParser, IsAArch32TripleWithMProfile) { +TEST_F(AProfileParser, IsAArch32TripleWithMProfile) { TargetJSON target = ParseTargetWithAttrs("cortex-m33", "armv7a-arm-none-eabi", {""}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), false); @@ -161,36 +195,35 @@ TEST(AProfileParser, IsAArch32TripleWithMProfile) { ASSERT_EQ(IsArch(target), false); } -TEST(AProfileParser, AArch64HasASIMD) { +TEST_F(AProfileParser, AArch64HasASIMD) { TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {""}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_asimd")), true); } -TEST(AProfileParser, AArch32ASIMD) { +TEST_F(AProfileParser, AArch32ASIMD) { TargetJSON target = ParseTargetWithAttrs("", "armv8a-arm-none-eabi", {}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); - // TODO(lhutton1) LLVM reports that Neon is support with this target string. Is that true? ASSERT_EQ(Downcast(features.at("has_asimd")), true); } -TEST(AProfileParser, AArch32HasASIMDWithOption) { +TEST_F(AProfileParser, AArch32HasASIMDWithOption) { TargetJSON target = ParseTargetWithAttrs("", "armv8a-arm-none-eabi", {"+simd"}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_asimd")), true); } -TEST(AProfileParser, AArch32HasASIMDWithAlternativeOption) { +TEST_F(AProfileParser, AArch32HasASIMDWithAlternativeOption) { TargetJSON target = ParseTargetWithAttrs("", "armv8a-arm-none-eabi", {"+neon"}); TargetFeatures features = Downcast(target.at("features")); ASSERT_EQ(IsArch(target), true); ASSERT_EQ(Downcast(features.at("has_asimd")), true); } -TEST(AProfileParser, DefaultI8MMSupport) { +TEST_F(AProfileParser, DefaultI8MMSupport) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultI8MM) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); @@ -198,7 +231,7 @@ TEST(AProfileParser, DefaultI8MMSupport) { ASSERT_EQ(Downcast(features.at("has_matmul_i8")), true); } -TEST(AProfileParser, DefaultI8MMSupportDisable) { +TEST_F(AProfileParser, DefaultI8MMSupportDisable) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultI8MM) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "-i8mm"}); TargetFeatures features = Downcast(target.at("features")); @@ -206,6 +239,7 @@ TEST(AProfileParser, DefaultI8MMSupportDisable) { ASSERT_EQ(Downcast(features.at("has_matmul_i8")), false); } +using AProfileOptionalI8MM = AProfileParserTestWithParam; TEST_P(AProfileOptionalI8MM, OptionalI8MMSupport) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; @@ -220,7 +254,7 @@ TEST_P(AProfileOptionalI8MM, OptionalI8MMSupport) { ASSERT_EQ(Downcast(features.at("has_matmul_i8")), true); } -TEST(AProfileParser, DefaultDotProdSupport) { +TEST_F(AProfileParser, DefaultDotProdSupport) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultDotProd) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); @@ -228,7 +262,7 @@ TEST(AProfileParser, DefaultDotProdSupport) { ASSERT_EQ(Downcast(features.at("has_dotprod")), true); } -TEST(AProfileParser, DefaultDotProdSupportDisable) { +TEST_F(AProfileParser, DefaultDotProdSupportDisable) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultDotProd) + "a"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr, "-dotprod"}); TargetFeatures features = Downcast(target.at("features")); @@ -236,6 +270,7 @@ TEST(AProfileParser, DefaultDotProdSupportDisable) { ASSERT_EQ(Downcast(features.at("has_dotprod")), false); } +using AProfileOptionalDotProd = AProfileParserTestWithParam; TEST_P(AProfileOptionalDotProd, OptionalDotProdSupport) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; @@ -250,7 +285,7 @@ TEST_P(AProfileOptionalDotProd, OptionalDotProdSupport) { ASSERT_EQ(Downcast(features.at("has_dotprod")), true); } -TEST(AProfileParser, ArchVersionInvalidLetter) { +TEST_F(AProfileParser, ArchVersionInvalidLetter) { std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(defaultDotProd) + "b"; TargetJSON target = ParseTargetWithAttrs("", "aarch64-arm-none-eabi", {arch_attr}); TargetFeatures features = Downcast(target.at("features")); @@ -258,7 +293,7 @@ TEST(AProfileParser, ArchVersionInvalidLetter) { ASSERT_EQ(Downcast(features.at("has_dotprod")), false); } -using AProfileOptionalSVE = testing::TestWithParam; +using AProfileOptionalSVE = AProfileParserTestWithParam; TEST_P(AProfileOptionalSVE, OptionalSVESupport) { const std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; @@ -275,7 +310,7 @@ TEST_P(AProfileOptionalSVE, OptionalSVESupport) { EXPECT_TRUE(Downcast(features.at("has_sve"))); } -TEST(AProfileDefaultSVE, DefaultSVESupportSVESupport) { +TEST_F(AProfileParser, DefaultSVESupportSVESupport) { const std::string arch_attr = "+v9a"; // Check that the "has_sve" feature is not set by default when "+sve" isn't set as an attribute. @@ -291,7 +326,7 @@ TEST(AProfileDefaultSVE, DefaultSVESupportSVESupport) { EXPECT_TRUE(Downcast(features.at("has_sve"))); } -using AProfileOptionalFP16 = testing::TestWithParam; +using AProfileOptionalFP16 = AProfileParserTestWithParam; TEST_P(AProfileOptionalFP16, OptionalFP16Support) { const std::string arch_attr = "+v" + FloatToStringWithoutTrailingZeros(GetParam()) + "a"; @@ -316,7 +351,7 @@ TEST_P(AProfileOptionalFP16, OptionalFP16Support) { EXPECT_TRUE(Downcast(features.at("has_fp16_simd"))); } -TEST(AProfileDefaultFP16, DefaultFP16Support) { +TEST_F(AProfileParser, DefaultFP16Support) { const std::string arch_attr = "+v9a"; // Check that the "has_fp16_simd" feature is not set by default when "+fullfp16" isn't set as an @@ -340,14 +375,14 @@ TEST(AProfileDefaultFP16, DefaultFP16Support) { EXPECT_TRUE(Downcast(features.at("has_fp16_simd"))); } -TEST(AProfileParser, ImpliedFeature) { +TEST_F(AProfileParser, ImpliedFeature) { TargetJSON target = ParseTargetWithAttrs("", "aarch64-linux-gnu", {"+sve2"}); TargetFeatures features = Downcast(target.at("features")); EXPECT_TRUE(Downcast(features.at("has_sve"))); EXPECT_TRUE(Downcast(features.at("has_asimd"))); } -TEST(AProfileParser, UnexpectedTargetKind) { +TEST_F(AProfileParser, UnexpectedTargetKind) { EXPECT_THROW( { try { @@ -360,15 +395,17 @@ TEST(AProfileParser, UnexpectedTargetKind) { tvm::InternalError); } -INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalI8MM, ::testing::ValuesIn(optionalI8MM)); -INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalDotProd, - ::testing::ValuesIn(optionalDotProd)); -INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalSVE, - ::testing::Values(8.0, 8.1, 8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9)); -INSTANTIATE_TEST_CASE_P(AProfileParser, AProfileOptionalFP16, - ::testing::Values(8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9)); +INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalI8MM, ::testing::ValuesIn(optionalI8MM)); +INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalDotProd, + ::testing::ValuesIn(optionalDotProd)); +INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalSVE, + ::testing::Values(8.0, 8.1, 8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9)); +INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalFP16, + ::testing::Values(8.2, 8.3, 8.4, 8.5, 8.6, 8.7, 8.8, 8.9)); } // namespace aprofile } // namespace parsers } // namespace target } // namespace tvm + +#endif From 5e6d9a6dad1e03f96ec96aa4faaf5a04137d758e Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Sat, 20 Jan 2024 14:30:26 +0000 Subject: [PATCH 3/9] Change python interface to return a set Change-Id: If41e4fd32947a2acddfe9b0691a0c9ba3245d722 --- python/tvm/target/codegen.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/tvm/target/codegen.py b/python/tvm/target/codegen.py index b2a92c2ca21b..82385e3b684f 100644 --- a/python/tvm/target/codegen.py +++ b/python/tvm/target/codegen.py @@ -183,7 +183,8 @@ def llvm_get_cpu_features(target=None): List of available CPU features. """ assert isinstance(target, Target) or target is None - return _ffi_api.llvm_get_cpu_features(target) + feature_map = _ffi_api.llvm_get_cpu_features(target) + return set(feature_map.keys()) def llvm_cpu_has_features(cpu_features, target=None): From 0d6a8b12a14a638e960da095fbfcec16b222f6fa Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Tue, 23 Jan 2024 14:03:04 +0000 Subject: [PATCH 4/9] Fix some more CI failures Notably, don't abort when encountering a CPU architecture that's not recognised by LLVM. This can happen when compiling with an older version of LLVM. Instead, output a warning. Also add additional checks in the parser for cases when TVM is not compiled with LLVM support and when LLVM is compiled without support for the necessary architectures. Change-Id: I646cb68cadd5462ee2bd694ba5c22ff7dad8f555 --- src/target/llvm/llvm_instance.cc | 7 ++- src/target/parsers/aprofile.cc | 24 +++++++++- tests/cpp/target/parsers/aprofile_test.cc | 48 +++++++++++-------- .../python/target/test_llvm_features_info.py | 15 ------ 4 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/target/llvm/llvm_instance.cc b/src/target/llvm/llvm_instance.cc index 765d7a5cf98b..cd39b465a4b9 100644 --- a/src/target/llvm/llvm_instance.cc +++ b/src/target/llvm/llvm_instance.cc @@ -221,8 +221,11 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const TargetJSON& target) bool has_arch = std::any_of(arches.begin(), arches.end(), [&](const auto& var) { return var == cpu_; }); if (!has_arch) { - LOG(FATAL) << "LLVM cpu architecture `-mcpu=" << cpu_ - << "` is not valid in `-mtriple=" << triple_ << "`"; + // Flag an error, but don't abort. This mimicks the behaviour of 'llc' to + // give the code a chance to run with a less-specific target. + LOG(ERROR) << "LLVM cpu architecture `-mcpu=" << cpu_ + << "` is not valid in `-mtriple=" << triple_ << "`" + << ", cpu architecture ignored"; } } diff --git a/src/target/parsers/aprofile.cc b/src/target/parsers/aprofile.cc index 524688a1d68a..5ee15994a8ba 100644 --- a/src/target/parsers/aprofile.cc +++ b/src/target/parsers/aprofile.cc @@ -76,14 +76,30 @@ bool IsArch(TargetJSON attrs) { return IsAArch32(mtriple, mcpu) || IsAArch64(mtriple); } +bool CheckContains(Array array, String predicate) { + return std::any_of(array.begin(), array.end(), [&](String var) { return var == predicate; }); +} + static TargetFeatures GetFeatures(TargetJSON target) { +#ifdef TVM_LLVM_VERSION String kind = Downcast(target.Get("kind")); ICHECK_EQ(kind, "llvm") << "Expected target kind 'llvm', but got '" << kind << "'"; Optional mtriple = Downcast>(target.Get("mtriple")); + Optional mcpu = Downcast>(target.Get("mcpu")); + + // Check that LLVM has been compiled with the correct target support + auto llvm_instance = std::make_unique(); + codegen::LLVMTargetInfo llvm_backend(*llvm_instance, "llvm"); + Array targets = llvm_backend.GetAllLLVMTargets(); + if ((IsAArch64(mtriple) && !CheckContains(targets, "aarch64")) || + (IsAArch32(mtriple, mcpu) && !CheckContains(targets, "arm"))) { + LOG(WARNING) << "Cannot parse target features. LLVM was not compiled with support for " + "Arm(R)-based targets."; + return {}; + } - auto llvm_instance = std::make_unique(); - tvm::codegen::LLVMTargetInfo llvm_target(*llvm_instance, target); + codegen::LLVMTargetInfo llvm_target(*llvm_instance, target); Map features = llvm_target.GetAllLLVMCpuFeatures(); auto has_feature = [features](const String& feature) { @@ -96,6 +112,10 @@ static TargetFeatures GetFeatures(TargetJSON target) { {"has_dotprod", Bool(has_feature("dotprod"))}, {"has_matmul_i8", Bool(has_feature("i8mm"))}, {"has_fp16_simd", Bool(has_feature("fullfp16"))}}; +#endif + + LOG(WARNING) << "Cannot parse Arm(R)-based target features without LLVM support."; + return {}; } static Array MergeKeys(Optional> existing_keys) { diff --git a/tests/cpp/target/parsers/aprofile_test.cc b/tests/cpp/target/parsers/aprofile_test.cc index d7dc063b320b..8972d5594514 100644 --- a/tests/cpp/target/parsers/aprofile_test.cc +++ b/tests/cpp/target/parsers/aprofile_test.cc @@ -17,8 +17,6 @@ * under the License. */ -#if TVM_LLVM_VERSION > 120 - #include "../src/target/parsers/aprofile.h" #include @@ -41,23 +39,25 @@ static float optionalI8MM[] = {8.2, 8.3, 8.4, 8.5}; static float defaultDotProd = 8.4; static float optionalDotProd[] = {8.2, 8.3}; -class AProfileParser : public ::testing::Test { - public: - AProfileParser() { - auto llvm_instance = std::make_unique(); - codegen::LLVMTargetInfo llvm_backend(*llvm_instance, "llvm"); - Array targets = llvm_backend.GetAllLLVMTargets(); - int expected_target_count = 0; - for (String target : targets) { - if (target == "aarch64" || target == "arm") { - expected_target_count += 1; - } - } - if (expected_target_count >= 2) { - has_aarch64_and_arm_targets = true; +static bool CheckArchitectureAvailability() { + auto llvm_instance = std::make_unique(); + codegen::LLVMTargetInfo llvm_backend(*llvm_instance, "llvm"); + Array targets = llvm_backend.GetAllLLVMTargets(); + int expected_target_count = 0; + for (String target : targets) { + if (target == "aarch64" || target == "arm") { + expected_target_count += 1; } } + if (expected_target_count >= 2) { + return true; + } + return false; +} +static bool has_aarch64_and_arm_targets = CheckArchitectureAvailability(); +class AProfileParser : public ::testing::Test { + public: // Check that LLVM has been compiled with the required targets, otherwise skip the test. // Unfortunately, googletest doesn't let you call GTEST_SKIP in SetUpTestSuite() to skip // the whole suite of tests, so a cached result is checked before each test is run instead. @@ -66,9 +66,6 @@ class AProfileParser : public ::testing::Test { GTEST_SKIP() << "Skipping as LLVM has not been built for Arm(R)-based targets."; } } - - private: - bool has_aarch64_and_arm_targets = false; }; class AProfileParserTestWithParam : public AProfileParser, @@ -395,6 +392,17 @@ TEST_F(AProfileParser, UnexpectedTargetKind) { tvm::InternalError); } +TEST(AProfileParserInvalid, LLVMUnsupportedArchitecture) { + if (has_aarch64_and_arm_targets) { + GTEST_SKIP() << "LLVM has been compiled for the correct targets."; + } + TargetJSON target = ParseTarget({{"kind", String("llvm")}}); + TargetFeatures features = Downcast(target.at("features")); + for (auto feature : features) { + ASSERT_EQ(Downcast(feature.second), false); + } +} + INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalI8MM, ::testing::ValuesIn(optionalI8MM)); INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalDotProd, ::testing::ValuesIn(optionalDotProd)); @@ -407,5 +415,3 @@ INSTANTIATE_TEST_SUITE_P(AProfileParser, AProfileOptionalFP16, } // namespace parsers } // namespace target } // namespace tvm - -#endif diff --git a/tests/python/target/test_llvm_features_info.py b/tests/python/target/test_llvm_features_info.py index edcbc891c90d..1760d31f71f4 100644 --- a/tests/python/target/test_llvm_features_info.py +++ b/tests/python/target/test_llvm_features_info.py @@ -39,21 +39,6 @@ def test_llvm_targets(): assert codegen.llvm_get_system_x86_vendor() == _ffi_api.llvm_get_system_x86_vendor() assert str(codegen.llvm_get_targets()) == str(_ffi_api.llvm_get_targets()) - # check LLVM target -mcpu legality - try: - tvm.target.codegen.llvm_get_cpu_features( - tvm.target.Target("llvm -mtriple=x86_64-linux-gnu -mcpu=dummy") - ) - assert False - except tvm.error.TVMError as e: - msg = str(e) - assert ( - msg.find( - "TVMError: LLVM cpu architecture `-mcpu=dummy` is not valid in `-mtriple=x86_64-linux-gnu`" - ) - != -1 - ) - min_llvm_version, llvm_target, cpu_arch, cpu_features, is_supported = tvm.testing.parameters( (-1, "x86_64", "sandybridge", "sse4.1", True), From 1ef3362e23261f8d195041073eb8bf03e1b6aee2 Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Tue, 23 Jan 2024 17:14:16 +0000 Subject: [PATCH 5/9] Check LLVM warning in target test Change-Id: Ibfd0beb6dda00aa2a93cd0b47cf28f045e3fde5c --- tests/python/target/test_llvm_features_info.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/python/target/test_llvm_features_info.py b/tests/python/target/test_llvm_features_info.py index 1760d31f71f4..0313662a108d 100644 --- a/tests/python/target/test_llvm_features_info.py +++ b/tests/python/target/test_llvm_features_info.py @@ -22,7 +22,7 @@ LLVM_VERSION = codegen.llvm_version_major() -def test_llvm_targets(): +def test_llvm_targets(capfd): ## ## check LLVM backend @@ -39,6 +39,13 @@ def test_llvm_targets(): assert codegen.llvm_get_system_x86_vendor() == _ffi_api.llvm_get_system_x86_vendor() assert str(codegen.llvm_get_targets()) == str(_ffi_api.llvm_get_targets()) + tvm.target.codegen.llvm_get_cpu_features( + tvm.target.Target("llvm -mtriple=x86_64-linux-gnu -mcpu=dummy") + ) + expected_str = ("Error: LLVM cpu architecture `-mcpu=dummy` is not valid in " + "`-mtriple=x86_64-linux-gnu`, cpu architecture ignored") + assert expected_str in capfd.readouterr().err + min_llvm_version, llvm_target, cpu_arch, cpu_features, is_supported = tvm.testing.parameters( (-1, "x86_64", "sandybridge", "sse4.1", True), From 1307306ec33d029c202e40c14e7df0b0a75e1893 Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Wed, 24 Jan 2024 10:05:04 +0000 Subject: [PATCH 6/9] Reintroduce LLVM version check in aprofile tests to fix i386 Change-Id: I0b88ecad2987297c428d0f0ca95db35d828c1672 --- tests/cpp/target/parsers/aprofile_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/cpp/target/parsers/aprofile_test.cc b/tests/cpp/target/parsers/aprofile_test.cc index 8972d5594514..a134e162fc2d 100644 --- a/tests/cpp/target/parsers/aprofile_test.cc +++ b/tests/cpp/target/parsers/aprofile_test.cc @@ -40,6 +40,7 @@ static float defaultDotProd = 8.4; static float optionalDotProd[] = {8.2, 8.3}; static bool CheckArchitectureAvailability() { +#if TVM_LLVM_VERSION > 120 auto llvm_instance = std::make_unique(); codegen::LLVMTargetInfo llvm_backend(*llvm_instance, "llvm"); Array targets = llvm_backend.GetAllLLVMTargets(); @@ -52,6 +53,7 @@ static bool CheckArchitectureAvailability() { if (expected_target_count >= 2) { return true; } +#endif return false; } static bool has_aarch64_and_arm_targets = CheckArchitectureAvailability(); From 877729d4d49becd3718d50b6566bb5987884d40c Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Mon, 29 Jan 2024 10:21:14 +0000 Subject: [PATCH 7/9] Use more specific warning message Change-Id: I98a72a95e2b51f8a4b577dcef15f40e7c28719a2 --- src/target/llvm/llvm_instance.cc | 2 +- .../relay/strategy/test_select_implementation.py | 10 ++++++++++ tests/python/target/test_llvm_features_info.py | 6 ++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/target/llvm/llvm_instance.cc b/src/target/llvm/llvm_instance.cc index cd39b465a4b9..4343db21069f 100644 --- a/src/target/llvm/llvm_instance.cc +++ b/src/target/llvm/llvm_instance.cc @@ -225,7 +225,7 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const TargetJSON& target) // give the code a chance to run with a less-specific target. LOG(ERROR) << "LLVM cpu architecture `-mcpu=" << cpu_ << "` is not valid in `-mtriple=" << triple_ << "`" - << ", cpu architecture ignored"; + << ", using default `-mcpu=" << String(defaults::cpu) << "`"; } } diff --git a/tests/python/relay/strategy/test_select_implementation.py b/tests/python/relay/strategy/test_select_implementation.py index 038c53ee65a1..d0767175d3d8 100644 --- a/tests/python/relay/strategy/test_select_implementation.py +++ b/tests/python/relay/strategy/test_select_implementation.py @@ -27,6 +27,7 @@ from tvm.relay.testing import run_infer_type, run_opt_pass import tvm.testing from tvm import topi +from tvm.target.codegen import llvm_version_major @pytest.mark.parametrize( @@ -90,6 +91,9 @@ def _get_conv2d_impl(dtype, target): return impl.name +@pytest.mark.skipif( + llvm_version_major() < 15, reason=f"Requires LLVM 15+, got {llvm_version_major()}" +) @pytest.mark.parametrize( "target,expected_impl", [ @@ -131,6 +135,9 @@ def test_int8_conv2d(target, expected_impl): assert selected_impl == expected_impl +@pytest.mark.skipif( + llvm_version_major() < 15, reason=f"Requires LLVM 15+, got {llvm_version_major()}" +) @pytest.mark.parametrize( "target,expected_impl", [ @@ -164,6 +171,9 @@ def test_fp32_conv2d(target, expected_impl): assert selected_impl == expected_impl +@pytest.mark.skipif( + llvm_version_major() < 15, reason=f"Requires LLVM 15+, got {llvm_version_major()}" +) @pytest.mark.parametrize( "target,expected_impl", [ diff --git a/tests/python/target/test_llvm_features_info.py b/tests/python/target/test_llvm_features_info.py index 0313662a108d..34e9a582313a 100644 --- a/tests/python/target/test_llvm_features_info.py +++ b/tests/python/target/test_llvm_features_info.py @@ -42,8 +42,10 @@ def test_llvm_targets(capfd): tvm.target.codegen.llvm_get_cpu_features( tvm.target.Target("llvm -mtriple=x86_64-linux-gnu -mcpu=dummy") ) - expected_str = ("Error: LLVM cpu architecture `-mcpu=dummy` is not valid in " - "`-mtriple=x86_64-linux-gnu`, cpu architecture ignored") + expected_str = ( + "Error: LLVM cpu architecture `-mcpu=dummy` is not valid in " + "`-mtriple=x86_64-linux-gnu`, using default `-mcpu=generic`" + ) assert expected_str in capfd.readouterr().err From 92c68f20e055ba07a6ad057628d7e23d21ca6f3f Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Mon, 18 Mar 2024 09:41:02 +0000 Subject: [PATCH 8/9] rebase to latest main Change-Id: Iac24bbe31251ebbafacb410abbb67f1e32c171d6 --- src/target/llvm/llvm_instance.cc | 2 +- src/target/parsers/aprofile.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target/llvm/llvm_instance.cc b/src/target/llvm/llvm_instance.cc index 4343db21069f..cbf3a2fa1c2a 100644 --- a/src/target/llvm/llvm_instance.cc +++ b/src/target/llvm/llvm_instance.cc @@ -262,7 +262,7 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance, const TargetJSON& target) } // LLVM JIT engine options - if (const Optional& v = target->GetAttr("jit")) { + if (const auto& v = Downcast>(target.Get("jit"))) { String value = v.value(); if ((value == "mcjit") || (value == "orcjit")) { jit_engine_ = value; diff --git a/src/target/parsers/aprofile.cc b/src/target/parsers/aprofile.cc index 5ee15994a8ba..907e0cae72d2 100644 --- a/src/target/parsers/aprofile.cc +++ b/src/target/parsers/aprofile.cc @@ -90,7 +90,7 @@ static TargetFeatures GetFeatures(TargetJSON target) { // Check that LLVM has been compiled with the correct target support auto llvm_instance = std::make_unique(); - codegen::LLVMTargetInfo llvm_backend(*llvm_instance, "llvm"); + codegen::LLVMTargetInfo llvm_backend(*llvm_instance, {{"kind", String("llvm")}}); Array targets = llvm_backend.GetAllLLVMTargets(); if ((IsAArch64(mtriple) && !CheckContains(targets, "aarch64")) || (IsAArch32(mtriple, mcpu) && !CheckContains(targets, "arm"))) { From 31927904b378c142be3044419e38501425805cd4 Mon Sep 17 00:00:00 2001 From: Luke Hutton Date: Mon, 25 Mar 2024 16:38:17 +0000 Subject: [PATCH 9/9] attempt to fix memory leak in llvm_instance.cc Change-Id: Ic8391063d403cc7fe7e2e7d1b4b3c2d6d3bc3146 --- src/target/llvm/llvm_instance.cc | 49 ++++++++++++-------------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/src/target/llvm/llvm_instance.cc b/src/target/llvm/llvm_instance.cc index cbf3a2fa1c2a..b3f55594a25f 100644 --- a/src/target/llvm/llvm_instance.cc +++ b/src/target/llvm/llvm_instance.cc @@ -386,41 +386,21 @@ static const llvm::Target* CreateLLVMTargetInstance(const std::string triple, return llvm_instance; } -static llvm::TargetMachine* CreateLLVMTargetMachine( +static std::unique_ptr CreateLLVMTargetMachine( const llvm::Target* llvm_instance, const std::string& triple, const std::string& cpu, - const std::string& features, const llvm::TargetOptions& target_options, - const llvm::Reloc::Model& reloc_model, const llvm::CodeModel::Model& code_model, + const std::string& features, const llvm::TargetOptions& target_options = {}, + const llvm::Reloc::Model& reloc_model = llvm::Reloc::Static, + const llvm::CodeModel::Model& code_model = llvm::CodeModel::Small, #if TVM_LLVM_VERSION <= 170 - const llvm::CodeGenOpt::Level& opt_level) { + const llvm::CodeGenOpt::Level& opt_level = llvm::CodeGenOpt::Level(0)) { #else - const llvm::CodeGenOptLevel& opt_level) { + const llvm::CodeGenOptLevel& opt_level = llvm::CodeGenOptLevel(0)) { #endif llvm::TargetMachine* tm = llvm_instance->createTargetMachine( triple, cpu, features, target_options, reloc_model, code_model, opt_level); ICHECK(tm != nullptr); - return tm; -} - -static const llvm::MCSubtargetInfo* GetLLVMSubtargetInfo(const std::string& triple, - const std::string& cpu_name, - const std::string& feats) { - // create a LLVM instance - auto llvm_instance = CreateLLVMTargetInstance(triple, true); - // create a target machine - // required minimum: llvm::InitializeAllTargetMCs() - llvm::TargetOptions target_options; - auto tm = CreateLLVMTargetMachine(llvm_instance, triple, cpu_name, feats, target_options, - llvm::Reloc::Static, llvm::CodeModel::Small, -#if TVM_LLVM_VERSION <= 170 - llvm::CodeGenOpt::Level(0)); -#else - llvm::CodeGenOptLevel(0)); -#endif - // create subtarget info module - const llvm::MCSubtargetInfo* MCInfo = tm->getMCSubtargetInfo(); - - return MCInfo; + return std::unique_ptr(tm); } llvm::TargetMachine* LLVMTargetInfo::GetOrCreateTargetMachine(bool allow_missing) { @@ -428,10 +408,9 @@ llvm::TargetMachine* LLVMTargetInfo::GetOrCreateTargetMachine(bool allow_missing std::string error; if (const llvm::Target* llvm_instance = CreateLLVMTargetInstance(triple_, allow_missing)) { - llvm::TargetMachine* tm = + target_machine_ = CreateLLVMTargetMachine(llvm_instance, triple_, cpu_, GetTargetFeatureString(), target_options_, reloc_model_, code_model_, opt_level_); - target_machine_ = std::unique_ptr(tm); } ICHECK(target_machine_ != nullptr); return target_machine_.get(); @@ -837,7 +816,11 @@ const Array LLVMTargetInfo::GetAllLLVMTargets() const { const Array LLVMTargetInfo::GetAllLLVMTargetArches() const { Array cpu_arches; // get the subtarget info module - const auto MCInfo = GetLLVMSubtargetInfo(triple_, "", ""); + auto llvm_instance = CreateLLVMTargetInstance(triple_, true); + std::unique_ptr target_machine = + CreateLLVMTargetMachine(llvm_instance, triple_, "", ""); + const auto MCInfo = target_machine->getMCSubtargetInfo(); + if (!MCInfo) { return cpu_arches; } @@ -861,7 +844,11 @@ const Map LLVMTargetInfo::GetAllLLVMCpuFeatures() const { feats += feats.empty() ? attr : ("," + attr); } // get the subtarget info module - const auto MCInfo = GetLLVMSubtargetInfo(triple_, cpu_.c_str(), feats); + auto llvm_instance = CreateLLVMTargetInstance(triple_, true); + std::unique_ptr target_machine = + CreateLLVMTargetMachine(llvm_instance, triple_, cpu_.c_str(), feats); + const auto MCInfo = target_machine->getMCSubtargetInfo(); + // get all features for CPU llvm::ArrayRef llvm_features = #if TVM_LLVM_VERSION < 180