ART: Enable ISA features run-time detection for ARM64

On a target run-time detected ISA features can be more accurate than
instruction set features based on a build-time information such as an
instruction set variant or C++ defines. Build-time based features can
be too generic and do not include all features a target CPU supports.

This CL enables instruction feature run-time detection in the JIT/AOT
compilers:

- The value "runtime" to the option "--instruction-set-features" to try
to detect CPU features at run time. If a target does not support run-time
detection it has the same effect as the value "default".
- Runtime uses "--instruction-set-features=runtime" if run-time detection is
supported.

The CL also cleans up how an instruction set feature string is processed
by InstructionSetFeatures::AddFeaturesFromString. It used to make redundant
uses of Trim in subclasses. The calls are replaced with DCHECKs
verifying that feature names are already trimmed.

Test: m test-art-target-gtest
Test: m test-art-host-gtest
Test: art/test.py --target --optimizing --interpreter --jit
Test: art/test.py --host --optimizing --interpreter --jit
Test: Pixel 3 UI booted

Change-Id: I223d5bc968d589dba5c09f6b03ee8c25987610b0

14 files changed, 361 insertions, 50 deletions

diff --git a/compiler/jit/jit_compiler.cc b/compiler/jit/jit_compiler.cc
index 0d35fecb7b..4d7ae9bd1b 100644
--- a/compiler/jit/jit_compiler.cc
+++ b/compiler/jit/jit_compiler.cc
@@ -99,7 +99,10 @@ void JitCompiler::ParseCompilerOptions() {
       }
     }
   }
+
   if (instruction_set_features == nullptr) {
+    // '--instruction-set-features/--instruction-set-variant' were not used.
+    // Use build-time defined features.
     instruction_set_features = InstructionSetFeatures::FromCppDefines();
   }
   compiler_options_->instruction_set_features_ = std::move(instruction_set_features);
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 35af918757..4e6210304a 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -296,6 +296,10 @@ NO_RETURN static void Usage(const char* fmt, ...) {
   UsageError("      Default: arm");
   UsageError("");
   UsageError("  --instruction-set-features=...,: Specify instruction set features");
+  UsageError("      On target the value 'runtime' can be used to detect features at run time.");
+  UsageError("      If target does not support run-time detection the value 'runtime'");
+  UsageError("      has the same effect as the value 'default'.");
+  UsageError("      Note: the value 'runtime' has no effect if it is used on host.");
   UsageError("      Example: --instruction-set-features=div");
   UsageError("      Default: default");
   UsageError("");
@@ -875,9 +879,9 @@ class Dex2Oat final {
       oat_unstripped_ = std::move(parser_options->oat_symbols);
     }
 
-    // If no instruction set feature was given, use the default one for the target
-    // instruction set.
-    if (compiler_options_->instruction_set_features_.get() == nullptr) {
+    if (compiler_options_->instruction_set_features_ == nullptr) {
+      // '--instruction-set-features/--instruction-set-variant' were not used.
+      // Use features for the 'default' variant.
       compiler_options_->instruction_set_features_ = InstructionSetFeatures::FromVariant(
           compiler_options_->instruction_set_, "default", &parser_options->error_msg);
       if (compiler_options_->instruction_set_features_ == nullptr) {
@@ -890,9 +894,9 @@ class Dex2Oat final {
       std::unique_ptr<const InstructionSetFeatures> runtime_features(
           InstructionSetFeatures::FromCppDefines());
       if (!compiler_options_->GetInstructionSetFeatures()->Equals(runtime_features.get())) {
-        LOG(WARNING) << "Mismatch between dex2oat instruction set features ("
+        LOG(WARNING) << "Mismatch between dex2oat instruction set features to use ("
             << *compiler_options_->GetInstructionSetFeatures()
-            << ") and those of dex2oat executable (" << *runtime_features
+            << ") and those from CPP defines (" << *runtime_features
             << ") for the command line:\n" << CommandLine();
       }
     }
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 524bce05b5..d3bfb57a0b 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <algorithm>
 #include <regex>
 #include <sstream>
 #include <string>
@@ -28,6 +29,7 @@
 
 #include "common_runtime_test.h"
 
+#include "arch/instruction_set_features.h"
 #include "base/macros.h"
 #include "base/mutex-inl.h"
 #include "base/utils.h"
@@ -2315,4 +2317,38 @@ TEST_F(Dex2oatClassLoaderContextTest, StoredClassLoaderContext) {
   }));
 }
 
+class Dex2oatISAFeaturesRuntimeDetectionTest : public Dex2oatTest {
+ protected:
+  void RunTest(const std::vector<std::string>& extra_args = {}) {
+    std::string dex_location = GetScratchDir() + "/Dex2OatSwapTest.jar";
+    std::string odex_location = GetOdexDir() + "/Dex2OatSwapTest.odex";
+
+    Copy(GetTestDexFileName(), dex_location);
+
+    ASSERT_TRUE(GenerateOdexForTest(dex_location,
+                                    odex_location,
+                                    CompilerFilter::kSpeed,
+                                    extra_args));
+  }
+
+  std::string GetTestDexFileName() {
+    return GetDexSrc1();
+  }
+};
+
+TEST_F(Dex2oatISAFeaturesRuntimeDetectionTest, TestCurrentRuntimeFeaturesAsDex2OatArguments) {
+  std::vector<std::string> argv;
+  Runtime::Current()->AddCurrentRuntimeFeaturesAsDex2OatArguments(&argv);
+  auto option_pos =
+      std::find(std::begin(argv), std::end(argv), "--instruction-set-features=runtime");
+  if (InstructionSetFeatures::IsRuntimeDetectionSupported()) {
+    EXPECT_TRUE(kIsTargetBuild);
+    EXPECT_NE(option_pos, std::end(argv));
+  } else {
+    EXPECT_EQ(option_pos, std::end(argv));
+  }
+
+  RunTest();
+}
+
 }  // namespace art
diff --git a/runtime/arch/arm/instruction_set_features_arm.cc b/runtime/arch/arm/instruction_set_features_arm.cc
index fcf3c756fb..fdf4dbd7be 100644
--- a/runtime/arch/arm/instruction_set_features_arm.cc
+++ b/runtime/arch/arm/instruction_set_features_arm.cc
@@ -319,8 +319,9 @@ ArmInstructionSetFeatures::AddFeaturesFromSplitString(
   bool has_atomic_ldrd_strd = has_atomic_ldrd_strd_;
   bool has_div = has_div_;
   bool has_armv8a = has_armv8a_;
-  for (auto i = features.begin(); i != features.end(); i++) {
-    std::string feature = android::base::Trim(*i);
+  for (const std::string& feature : features) {
+    DCHECK_EQ(android::base::Trim(feature), feature)
+        << "Feature name is not trimmed: '" << feature << "'";
     if (feature == "div") {
       has_div = true;
     } else if (feature == "-div") {
diff --git a/runtime/arch/arm64/instruction_set_features_arm64.cc b/runtime/arch/arm64/instruction_set_features_arm64.cc
index 4a2b9d59e8..196f35871b 100644
--- a/runtime/arch/arm64/instruction_set_features_arm64.cc
+++ b/runtime/arch/arm64/instruction_set_features_arm64.cc
@@ -315,8 +315,9 @@ Arm64InstructionSetFeatures::AddFeaturesFromSplitString(
   bool has_lse = has_lse_;
   bool has_fp16 = has_fp16_;
   bool has_dotprod = has_dotprod_;
-  for (auto i = features.begin(); i != features.end(); i++) {
-    std::string feature = android::base::Trim(*i);
+  for (const std::string& feature : features) {
+    DCHECK_EQ(android::base::Trim(feature), feature)
+        << "Feature name is not trimmed: '" << feature << "'";
     if (feature == "a53") {
       is_a53 = true;
     } else if (feature == "-a53") {
@@ -367,4 +368,17 @@ Arm64InstructionSetFeatures::AddFeaturesFromSplitString(
                                       has_dotprod));
 }
 
+std::unique_ptr<const InstructionSetFeatures>
+Arm64InstructionSetFeatures::AddRuntimeDetectedFeatures(
+    const InstructionSetFeatures *features) const {
+  const Arm64InstructionSetFeatures *arm64_features = features->AsArm64InstructionSetFeatures();
+  return std::unique_ptr<const InstructionSetFeatures>(
+      new Arm64InstructionSetFeatures(fix_cortex_a53_835769_,
+                                      fix_cortex_a53_843419_,
+                                      arm64_features->has_crc_,
+                                      arm64_features->has_lse_,
+                                      arm64_features->has_fp16_,
+                                      arm64_features->has_dotprod_));
+}
+
 }  // namespace art
diff --git a/runtime/arch/arm64/instruction_set_features_arm64.h b/runtime/arch/arm64/instruction_set_features_arm64.h
index 4ec8fa2362..432b9ef6f2 100644
--- a/runtime/arch/arm64/instruction_set_features_arm64.h
+++ b/runtime/arch/arm64/instruction_set_features_arm64.h
@@ -98,6 +98,9 @@ class Arm64InstructionSetFeatures final : public InstructionSetFeatures {
       AddFeaturesFromSplitString(const std::vector<std::string>& features,
                                  std::string* error_msg) const override;
 
+  std::unique_ptr<const InstructionSetFeatures>
+      AddRuntimeDetectedFeatures(const InstructionSetFeatures *features) const override;
+
  private:
   Arm64InstructionSetFeatures(bool needs_a53_835769_fix,
                               bool needs_a53_843419_fix,
diff --git a/runtime/arch/arm64/instruction_set_features_arm64_test.cc b/runtime/arch/arm64/instruction_set_features_arm64_test.cc
index 99d6b0dc59..eef8f086af 100644
--- a/runtime/arch/arm64/instruction_set_features_arm64_test.cc
+++ b/runtime/arch/arm64/instruction_set_features_arm64_test.cc
@@ -170,4 +170,54 @@ TEST(Arm64InstructionSetFeaturesTest, Arm64AddFeaturesFromString) {
   EXPECT_EQ(armv8_2a_cpu_features->AsBitmap(), 14U);
 }
 
+TEST(Arm64InstructionSetFeaturesTest, IsRuntimeDetectionSupported) {
+  if (kRuntimeISA == InstructionSet::kArm64) {
+    EXPECT_TRUE(InstructionSetFeatures::IsRuntimeDetectionSupported());
+  }
+}
+
+TEST(Arm64InstructionSetFeaturesTest, FeaturesFromRuntimeDetection) {
+  if (kRuntimeISA != InstructionSet::kArm64) {
+    return;
+  }
+
+  std::unique_ptr<const InstructionSetFeatures> hwcap_features(
+      InstructionSetFeatures::FromHwcap());
+  std::unique_ptr<const InstructionSetFeatures> runtime_detected_features(
+      InstructionSetFeatures::FromRuntimeDetection());
+  std::unique_ptr<const InstructionSetFeatures> cpp_defined_features(
+      InstructionSetFeatures::FromCppDefines());
+  EXPECT_NE(runtime_detected_features, nullptr);
+  EXPECT_TRUE(InstructionSetFeatures::IsRuntimeDetectionSupported());
+  EXPECT_TRUE(runtime_detected_features->Equals(hwcap_features.get()));
+  EXPECT_TRUE(runtime_detected_features->HasAtLeast(cpp_defined_features.get()));
+}
+
+TEST(Arm64InstructionSetFeaturesTest, AddFeaturesFromStringRuntime) {
+  std::unique_ptr<const InstructionSetFeatures> features(
+      InstructionSetFeatures::FromBitmap(InstructionSet::kArm64, 0x0));
+  std::unique_ptr<const InstructionSetFeatures> hwcap_features(
+      InstructionSetFeatures::FromHwcap());
+
+  std::string error_msg;
+  features = features->AddFeaturesFromString("runtime", &error_msg);
+
+  EXPECT_NE(features, nullptr);
+  EXPECT_TRUE(error_msg.empty());
+
+  if (kRuntimeISA == InstructionSet::kArm64) {
+    EXPECT_TRUE(features->Equals(hwcap_features.get()));
+    EXPECT_EQ(features->GetFeatureString(), hwcap_features->GetFeatureString());
+  }
+
+  std::unique_ptr<const InstructionSetFeatures> a53_features(
+      Arm64InstructionSetFeatures::FromVariant("cortex-a53", &error_msg));
+  features = a53_features->AddFeaturesFromString("runtime", &error_msg);
+  EXPECT_NE(features, nullptr);
+  EXPECT_TRUE(error_msg.empty()) << error_msg;
+  const Arm64InstructionSetFeatures *arm64_features = features->AsArm64InstructionSetFeatures();
+  EXPECT_TRUE(arm64_features->NeedFixCortexA53_835769());
+  EXPECT_TRUE(arm64_features->NeedFixCortexA53_843419());
+}
+
 }  // namespace art
diff --git a/runtime/arch/instruction_set_features.cc b/runtime/arch/instruction_set_features.cc
index 886b40af30..c5c2d31158 100644
--- a/runtime/arch/instruction_set_features.cc
+++ b/runtime/arch/instruction_set_features.cc
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <algorithm>
+
 #include "instruction_set_features.h"
 
 #include <algorithm>
@@ -113,6 +115,16 @@ std::unique_ptr<const InstructionSetFeatures> InstructionSetFeatures::FromCppDef
   UNREACHABLE();
 }
 
+std::unique_ptr<const InstructionSetFeatures> InstructionSetFeatures::FromRuntimeDetection() {
+  switch (kRuntimeISA) {
+#ifdef ART_TARGET_ANDROID
+    case InstructionSet::kArm64:
+      return Arm64InstructionSetFeatures::FromHwcap();
+#endif
+    default:
+      return nullptr;
+  }
+}
 
 std::unique_ptr<const InstructionSetFeatures> InstructionSetFeatures::FromCpuInfo() {
   switch (kRuntimeISA) {
@@ -184,44 +196,57 @@ std::unique_ptr<const InstructionSetFeatures> InstructionSetFeatures::FromAssemb
 }
 
 std::unique_ptr<const InstructionSetFeatures> InstructionSetFeatures::AddFeaturesFromString(
-    const std::string& feature_list, std::string* error_msg) const {
-  if (feature_list.empty()) {
-    *error_msg = "No instruction set features specified";
-    return std::unique_ptr<const InstructionSetFeatures>();
-  }
+    const std::string& feature_list, /* out */ std::string* error_msg) const {
   std::vector<std::string> features;
   Split(feature_list, ',', &features);
-  bool use_default = false;  // Have we seen the 'default' feature?
-  bool first = false;  // Is this first feature?
-  for (auto it = features.begin(); it != features.end();) {
-    if (use_default) {
-      *error_msg = "Unexpected instruction set features after 'default'";
-      return std::unique_ptr<const InstructionSetFeatures>();
-    }
-    std::string feature = android::base::Trim(*it);
-    bool erase = false;
+  std::transform(std::begin(features), std::end(features), std::begin(features),
+                 [](const std::string &s) { return android::base::Trim(s); });
+  auto empty_strings_begin = std::copy_if(std::begin(features), std::end(features),
+                                          std::begin(features),
+                                          [](const std::string& s) { return !s.empty(); });
+  features.erase(empty_strings_begin, std::end(features));
+  if (features.empty()) {
+    *error_msg = "No instruction set features specified";
+    return nullptr;
+  }
+
+  bool use_default = false;
+  bool use_runtime_detection = false;
+  for (const std::string& feature : features) {
     if (feature == "default") {
-      if (!first) {
-        use_default = true;
-        erase = true;
-      } else {
-        *error_msg = "Unexpected instruction set features before 'default'";
-        return std::unique_ptr<const InstructionSetFeatures>();
+      if (features.size() > 1) {
+        *error_msg = "Specific instruction set feature(s) cannot be used when 'default' is used.";
+        return nullptr;
       }
+      use_default = true;
+      features.pop_back();
+      break;
+    } else if (feature == "runtime") {
+      if (features.size() > 1) {
+        *error_msg = "Specific instruction set feature(s) cannot be used when 'runtime' is used.";
+        return nullptr;
+      }
+      use_runtime_detection = true;
+      features.pop_back();
+      break;
     }
-    if (!erase) {
-      ++it;
-    } else {
-      it = features.erase(it);
-    }
-    first = true;
   }
-  // Expectation: "default" is standalone, no other flags. But an empty features vector after
-  // processing can also come along if the handled flags are the only ones in the list. So
-  // logically, we check "default -> features.empty."
-  DCHECK(!use_default || features.empty());
+  // Expectation: "default" and "runtime" are standalone, no other feature names.
+  // But an empty features vector after processing can also come along if the
+  // handled feature names  are the only ones in the list. So
+  // logically, we check "default or runtime => features.empty."
+  DCHECK((!use_default && !use_runtime_detection) || features.empty());
+
+  std::unique_ptr<const InstructionSetFeatures> runtime_detected_features;
+  if (use_runtime_detection) {
+    runtime_detected_features = FromRuntimeDetection();
+  }
 
-  return AddFeaturesFromSplitString(features, error_msg);
+  if (runtime_detected_features != nullptr) {
+    return AddRuntimeDetectedFeatures(runtime_detected_features.get());
+  } else {
+    return AddFeaturesFromSplitString(features, error_msg);
+  }
 }
 
 const ArmInstructionSetFeatures* InstructionSetFeatures::AsArmInstructionSetFeatures() const {
@@ -262,6 +287,12 @@ bool InstructionSetFeatures::FindVariantInArray(const char* const variants[], si
   return std::find(begin, end, variant) != end;
 }
 
+std::unique_ptr<const InstructionSetFeatures> InstructionSetFeatures::AddRuntimeDetectedFeatures(
+    const InstructionSetFeatures *features ATTRIBUTE_UNUSED) const {
+  UNIMPLEMENTED(FATAL) << kRuntimeISA;
+  UNREACHABLE();
+}
+
 std::ostream& operator<<(std::ostream& os, const InstructionSetFeatures& rhs) {
   os << "ISA: " << rhs.GetInstructionSet() << " Feature string: " << rhs.GetFeatureString();
   return os;
diff --git a/runtime/arch/instruction_set_features.h b/runtime/arch/instruction_set_features.h
index f910a4183d..9222a7bad7 100644
--- a/runtime/arch/instruction_set_features.h
+++ b/runtime/arch/instruction_set_features.h
@@ -48,6 +48,20 @@ class InstructionSetFeatures {
   // Turn C pre-processor #defines into the equivalent instruction set features for kRuntimeISA.
   static std::unique_ptr<const InstructionSetFeatures> FromCppDefines();
 
+  // Check if run-time detection of instruction set features is supported.
+  //
+  // Return: true - if run-time detection is supported on a target device.
+  //         false - otherwise
+  static bool IsRuntimeDetectionSupported() {
+    return FromRuntimeDetection() != nullptr;
+  }
+
+  // Use run-time detection to get instruction set features.
+  //
+  // Return: a set of detected features or nullptr if runtime detection is not
+  //         supported on a target.
+  static std::unique_ptr<const InstructionSetFeatures> FromRuntimeDetection();
+
   // Process /proc/cpuinfo and use kRuntimeISA to produce InstructionSetFeatures.
   static std::unique_ptr<const InstructionSetFeatures> FromCpuInfo();
 
@@ -126,6 +140,10 @@ class InstructionSetFeatures {
       AddFeaturesFromSplitString(const std::vector<std::string>& features,
                                  std::string* error_msg) const = 0;
 
+  // Add run-time detected architecture specific features in sub-classes.
+  virtual std::unique_ptr<const InstructionSetFeatures>
+      AddRuntimeDetectedFeatures(const InstructionSetFeatures *features ATTRIBUTE_UNUSED) const;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(InstructionSetFeatures);
 };
diff --git a/runtime/arch/instruction_set_features_test.cc b/runtime/arch/instruction_set_features_test.cc
index 3a39a2a523..d9b2e3ffa1 100644
--- a/runtime/arch/instruction_set_features_test.cc
+++ b/runtime/arch/instruction_set_features_test.cc
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <array>
+
 #include "instruction_set_features.h"
 
 #include <gtest/gtest.h>
@@ -161,4 +163,145 @@ TEST(InstructionSetFeaturesTest, FeaturesFromAssembly) {
       << "\nFeatures from build: " << *instruction_set_features.get();
 }
 
+TEST(InstructionSetFeaturesTest, FeaturesFromRuntimeDetection) {
+  if (!InstructionSetFeatures::IsRuntimeDetectionSupported()) {
+    EXPECT_EQ(InstructionSetFeatures::FromRuntimeDetection(), nullptr);
+  }
+}
+
+// The instruction set feature string must not contain 'default' together with
+// other feature names.
+//
+// Test that InstructionSetFeatures::AddFeaturesFromString returns nullptr and
+// an error is reported when the value 'default' is specified together
+// with other feature names in an instruction set feature string.
+TEST(InstructionSetFeaturesTest, AddFeaturesFromStringWithDefaultAndOtherNames) {
+  std::unique_ptr<const InstructionSetFeatures> cpp_defined_features(
+      InstructionSetFeatures::FromCppDefines());
+  std::vector<std::string> invalid_feature_strings = {
+    "a,default",
+    "default,a",
+    "a,default,b",
+    "a,b,default",
+    "default,a,b,c",
+    "a,b,default,c,d",
+    "a, default ",
+    " default , a",
+    "a, default , b",
+    "default,runtime"
+  };
+
+  for (const std::string& invalid_feature_string : invalid_feature_strings) {
+    std::string error_msg;
+    EXPECT_EQ(cpp_defined_features->AddFeaturesFromString(invalid_feature_string, &error_msg),
+              nullptr) << " Invalid feature string: '" << invalid_feature_string << "'";
+    EXPECT_EQ(error_msg,
+              "Specific instruction set feature(s) cannot be used when 'default' is used.");
+  }
+}
+
+// The instruction set feature string must not contain 'runtime' together with
+// other feature names.
+//
+// Test that InstructionSetFeatures::AddFeaturesFromString returns nullptr and
+// an error is reported when the value 'runtime' is specified together
+// with other feature names in an instruction set feature string.
+TEST(InstructionSetFeaturesTest, AddFeaturesFromStringWithRuntimeAndOtherNames) {
+  std::unique_ptr<const InstructionSetFeatures> cpp_defined_features(
+      InstructionSetFeatures::FromCppDefines());
+  std::vector<std::string> invalid_feature_strings = {
+    "a,runtime",
+    "runtime,a",
+    "a,runtime,b",
+    "a,b,runtime",
+    "runtime,a,b,c",
+    "a,b,runtime,c,d",
+    "a, runtime ",
+    " runtime , a",
+    "a, runtime , b",
+    "runtime,default"
+  };
+
+  for (const std::string& invalid_feature_string : invalid_feature_strings) {
+    std::string error_msg;
+    EXPECT_EQ(cpp_defined_features->AddFeaturesFromString(invalid_feature_string, &error_msg),
+              nullptr) << " Invalid feature string: '" << invalid_feature_string << "'";
+    EXPECT_EQ(error_msg,
+              "Specific instruction set feature(s) cannot be used when 'runtime' is used.");
+  }
+}
+
+// Spaces and multiple commas are ignores in a instruction set feature string.
+//
+// Test that a use of spaces and multiple commas with 'default' and 'runtime'
+// does not cause errors.
+TEST(InstructionSetFeaturesTest, AddFeaturesFromValidStringContainingDefaultOrRuntime) {
+  std::unique_ptr<const InstructionSetFeatures> cpp_defined_features(
+      InstructionSetFeatures::FromCppDefines());
+  std::vector<std::string> valid_feature_strings = {
+    "default",
+    ",,,default",
+    "default,,,,",
+    ",,,default,,,,",
+    "default, , , ",
+    " , , ,default",
+    " , , ,default, , , ",
+    " default , , , ",
+    ",,,runtime",
+    "runtime,,,,",
+    ",,,runtime,,,,",
+    "runtime, , , ",
+    " , , ,runtime",
+    " , , ,runtime, , , ",
+    " runtime , , , "
+  };
+  for (const std::string& valid_feature_string : valid_feature_strings) {
+    std::string error_msg;
+    EXPECT_NE(cpp_defined_features->AddFeaturesFromString(valid_feature_string, &error_msg),
+              nullptr) << " Valid feature string: '" << valid_feature_string << "'";
+    EXPECT_TRUE(error_msg.empty()) << error_msg;
+  }
+}
+
+// Spaces and multiple commas are ignores in a instruction set feature string.
+//
+// Test that a use of spaces and multiple commas without any feature names
+// causes errors.
+TEST(InstructionSetFeaturesTest, AddFeaturesFromInvalidStringWithoutFeatureNames) {
+  std::unique_ptr<const InstructionSetFeatures> cpp_defined_features(
+      InstructionSetFeatures::FromCppDefines());
+  std::vector<std::string> invalid_feature_strings = {
+    " ",
+    "       ",
+    ",",
+    ",,",
+    " , , ,,,,,,",
+    "\t",
+    "  \t     ",
+    ",",
+    ",,",
+    " , , ,,,,,,"
+  };
+  for (const std::string& invalid_feature_string : invalid_feature_strings) {
+    std::string error_msg;
+    EXPECT_EQ(cpp_defined_features->AddFeaturesFromString(invalid_feature_string, &error_msg),
+              nullptr) << " Invalid feature string: '" << invalid_feature_string << "'";
+    EXPECT_EQ(error_msg, "No instruction set features specified");
+  }
+}
+
+TEST(InstructionSetFeaturesTest, AddFeaturesFromStringRuntime) {
+  std::unique_ptr<const InstructionSetFeatures> cpp_defined_features(
+      InstructionSetFeatures::FromCppDefines());
+  std::string error_msg;
+
+  const std::unique_ptr<const InstructionSetFeatures> features =
+      cpp_defined_features->AddFeaturesFromString("runtime", &error_msg);
+  EXPECT_NE(features, nullptr);
+  EXPECT_TRUE(error_msg.empty()) << error_msg;
+  if (!InstructionSetFeatures::IsRuntimeDetectionSupported()) {
+    EXPECT_TRUE(features->Equals(cpp_defined_features.get()));
+  }
+}
+
 }  // namespace art
diff --git a/runtime/arch/mips/instruction_set_features_mips.cc b/runtime/arch/mips/instruction_set_features_mips.cc
index 952ed250d2..99ce53638a 100644
--- a/runtime/arch/mips/instruction_set_features_mips.cc
+++ b/runtime/arch/mips/instruction_set_features_mips.cc
@@ -214,8 +214,9 @@ MipsInstructionSetFeatures::AddFeaturesFromSplitString(
   bool mips_isa_gte2 = mips_isa_gte2_;
   bool r6 = r6_;
   bool msa = msa_;
-  for (auto i = features.begin(); i != features.end(); i++) {
-    std::string feature = android::base::Trim(*i);
+  for (const std::string& feature : features) {
+    DCHECK_EQ(android::base::Trim(feature), feature)
+        << "Feature name is not trimmed: '" << feature << "'";
     if (feature == "fpu32") {
       fpu_32bit = true;
     } else if (feature == "-fpu32") {
diff --git a/runtime/arch/mips64/instruction_set_features_mips64.cc b/runtime/arch/mips64/instruction_set_features_mips64.cc
index ea9f84bd2d..2031433321 100644
--- a/runtime/arch/mips64/instruction_set_features_mips64.cc
+++ b/runtime/arch/mips64/instruction_set_features_mips64.cc
@@ -114,8 +114,9 @@ std::unique_ptr<const InstructionSetFeatures>
 Mips64InstructionSetFeatures::AddFeaturesFromSplitString(
     const std::vector<std::string>& features, std::string* error_msg) const {
   bool msa = msa_;
-  for (auto i = features.begin(); i != features.end(); i++) {
-    std::string feature = android::base::Trim(*i);
+  for (const std::string& feature : features) {
+    DCHECK_EQ(android::base::Trim(feature), feature)
+        << "Feature name is not trimmed: '" << feature << "'";
     if (feature == "msa") {
       msa = true;
     } else if (feature == "-msa") {
diff --git a/runtime/arch/x86/instruction_set_features_x86.cc b/runtime/arch/x86/instruction_set_features_x86.cc
index e9e983cda2..0c3d26e170 100644
--- a/runtime/arch/x86/instruction_set_features_x86.cc
+++ b/runtime/arch/x86/instruction_set_features_x86.cc
@@ -311,8 +311,9 @@ std::unique_ptr<const InstructionSetFeatures> X86InstructionSetFeatures::AddFeat
   bool has_AVX = has_AVX_;
   bool has_AVX2 = has_AVX2_;
   bool has_POPCNT = has_POPCNT_;
-  for (auto i = features.begin(); i != features.end(); i++) {
-    std::string feature = android::base::Trim(*i);
+  for (const std::string& feature : features) {
+    DCHECK_EQ(android::base::Trim(feature), feature)
+        << "Feature name is not trimmed: '" << feature << "'";
     if (feature == "ssse3") {
       has_SSSE3 = true;
     } else if (feature == "-ssse3") {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index feade83a2e..26f21b0c9f 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -2475,10 +2475,15 @@ void Runtime::AddCurrentRuntimeFeaturesAsDex2OatArguments(std::vector<std::strin
   instruction_set += GetInstructionSetString(kRuntimeISA);
   argv->push_back(instruction_set);
 
-  std::unique_ptr<const InstructionSetFeatures> features(InstructionSetFeatures::FromCppDefines());
-  std::string feature_string("--instruction-set-features=");
-  feature_string += features->GetFeatureString();
-  argv->push_back(feature_string);
+  if (InstructionSetFeatures::IsRuntimeDetectionSupported()) {
+    argv->push_back("--instruction-set-features=runtime");
+  } else {
+    std::unique_ptr<const InstructionSetFeatures> features(
+        InstructionSetFeatures::FromCppDefines());
+    std::string feature_string("--instruction-set-features=");
+    feature_string += features->GetFeatureString();
+    argv->push_back(feature_string);
+  }
 }
 
 void Runtime::CreateJitCodeCache(bool rwx_memory_allowed) {