MIPS32: Fix MipsInstructionSetFeatures::FromVariant()

Make it possible to completely override build-time instruction
set features with the provided variant string. Add sanity checks.
Comment on the "default" variant behavior.

Tested:
- test-art-host-gtest
- test-art-target-gtest and test-art-target-run-test-optimizing on:
  - MIPS32R6 (2nd arch) QEMU
  - CI20 board

Change-Id: I2470b7115a5b5e333e2e7a156e68c39945fb02e9

diff --git a/runtime/arch/mips/instruction_set_features_mips.cc b/runtime/arch/mips/instruction_set_features_mips.cc
index 93d79b7..b3a9866 100644 (file)
--- a/runtime/arch/mips/instruction_set_features_mips.cc
+++ b/runtime/arch/mips/instruction_set_features_mips.cc
@@ -76,21 +76,22 @@ const MipsInstructionSetFeatures* MipsInstructionSetFeatures::FromVariant(
   GetFlagsFromCppDefined(&mips_isa_gte2, &r6, &fpu_32bit);
 
   // Override defaults based on variant string.
-  // Only care if it is R1, R2 or R6 and we assume all CPUs will have a FP unit.
+  // Only care if it is R1, R2, R5 or R6 and we assume all CPUs will have a FP unit.
   constexpr const char* kMips32Prefix = "mips32r";
   const size_t kPrefixLength = strlen(kMips32Prefix);
   if (variant.compare(0, kPrefixLength, kMips32Prefix, kPrefixLength) == 0 &&
       variant.size() > kPrefixLength) {
-    if (variant[kPrefixLength] >= '6') {
-      fpu_32bit = false;
-      r6 = true;
-    }
-    if (variant[kPrefixLength] >= '2') {
-      mips_isa_gte2 = true;
-    }
+    r6 = (variant[kPrefixLength] >= '6');
+    fpu_32bit = (variant[kPrefixLength] < '5');
+    mips_isa_gte2 = (variant[kPrefixLength] >= '2');
   } else if (variant == "default") {
-    // Default variant is: smp = true, has fpu, is gte2, is not r6. This is the traditional
-    // setting.
+    // Default variant is: smp = true, has FPU, is gte2. This is the traditional setting.
+    //
+    // Note, we get FPU bitness and R6-ness from the build (using cpp defines, see above)
+    // and don't override them because many things depend on the "default" variant being
+    // sufficient for most purposes. That is, "default" should work for both R2 and R6.
+    // Use "mips32r#" to get a specific configuration, possibly not matching the runtime
+    // ISA (e.g. for ISA-specific testing of dex2oat internals).
     mips_isa_gte2 = true;
   } else {
     LOG(WARNING) << "Unexpected CPU variant for Mips32 using defaults: " << variant;

diff --git a/runtime/arch/mips/instruction_set_features_mips.h b/runtime/arch/mips/instruction_set_features_mips.h
index aac436e..120dc1c 100644 (file)
--- a/runtime/arch/mips/instruction_set_features_mips.h
+++ b/runtime/arch/mips/instruction_set_features_mips.h
@@ -81,8 +81,19 @@ class MipsInstructionSetFeatures FINAL : public InstructionSetFeatures {
 
  private:
   MipsInstructionSetFeatures(bool smp, bool fpu_32bit, bool mips_isa_gte2, bool r6)
-      : InstructionSetFeatures(smp), fpu_32bit_(fpu_32bit),  mips_isa_gte2_(mips_isa_gte2), r6_(r6)
-  {}
+      : InstructionSetFeatures(smp),
+        fpu_32bit_(fpu_32bit),
+        mips_isa_gte2_(mips_isa_gte2),
+        r6_(r6) {
+    // Sanity checks.
+    if (r6) {
+      CHECK(mips_isa_gte2);
+      CHECK(!fpu_32bit);
+    }
+    if (!mips_isa_gte2) {
+      CHECK(fpu_32bit);
+    }
+  }
 
   // Bitmap positions for encoding features as a bitmap.
   enum {