[arm] Simplify run-time CPU selection.

src/arm/assembler-arm.cc

Index: src/arm/assembler-arm.cc
diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index 78ffe25390592406cf51c78f21f5ab466b2c99fb..90ca15a8849c287a971fddb5127a7f12f28b9847 100644
--- a/src/arm/assembler-arm.cc
+++ b/src/arm/assembler-arm.cc
@@ -46,84 +46,116 @@
 namespace v8 {
 namespace internal {
 
-// Get the CPU features enabled by the build. For cross compilation the
-// preprocessor symbols CAN_USE_ARMV7_INSTRUCTIONS and CAN_USE_VFP3_INSTRUCTIONS
-// can be defined to enable ARMv7 and VFPv3 instructions when building the
-// snapshot.
-static unsigned CpuFeaturesImpliedByCompiler() {
-  unsigned answer = 0;
-#ifdef CAN_USE_ARMV8_INSTRUCTIONS
-  if (FLAG_enable_armv8) {
-    answer |= 1u << ARMv8;
-    // ARMv8 always features VFP and NEON.
-    answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
-    answer |= 1u << SUDIV;
+static const unsigned kArmv6 = 0u;
+static const unsigned kArmv7 = kArmv6 | (1u << ARMv7);
+static const unsigned kArmv7WithSudiv = kArmv7 | (1u << ARMv7_SUDIV);
+static const unsigned kArmv8 = kArmv7WithSudiv | (1u << ARMv8);
+
+static unsigned CpuFeaturesFromCommandLine() {
+  if (strcmp(FLAG_arm_arch, "armv8") == 0) {
+    return kArmv8;
+  } else if (strcmp(FLAG_arm_arch, "armv7+sudiv") == 0) {
+    return kArmv7WithSudiv;
+  } else if (strcmp(FLAG_arm_arch, "armv7") == 0) {
+    return kArmv7;
+  } else if (strcmp(FLAG_arm_arch, "armv6") == 0) {
+    return kArmv6;
   }
-#endif  // CAN_USE_ARMV8_INSTRUCTIONS
-#ifdef CAN_USE_ARMV7_INSTRUCTIONS
-  if (FLAG_enable_armv7) answer |= 1u << ARMv7;
-#endif  // CAN_USE_ARMV7_INSTRUCTIONS
-#ifdef CAN_USE_VFP3_INSTRUCTIONS
-  if (FLAG_enable_vfp3) answer |= 1u << VFP3 | 1u << ARMv7;
-#endif  // CAN_USE_VFP3_INSTRUCTIONS
-#ifdef CAN_USE_VFP32DREGS
-  if (FLAG_enable_32dregs) answer |= 1u << VFP32DREGS;
-#endif  // CAN_USE_VFP32DREGS
-#ifdef CAN_USE_NEON
-  if (FLAG_enable_neon) answer |= 1u << NEON;
-#endif  // CAN_USE_VFP32DREGS
 
-  return answer;
+  fprintf(stderr, "Warning: unrecognised value for --arm-arch ('%s').\n",

[Michael Achenbach, 2016/08/12 10:16:37]

I suggest we raise an error here since the flag is new. If some bot or dev by
misconfiguration (e.g. small typo in the string) runs tests with an unknown
arch, we'd (rather silently) test the wrong thing. E.g. the test driver doesn't
show stdout of single tests (unless they fail). If they print a warning, nobody
detects that.

+          FLAG_arm_arch);
+  fprintf(stderr,
+          "Supported values are:  armv8\n"
+          "                       armv7+sudiv\n"
+          "                       armv7\n"
+          "                       armv6\n"
+          "Falling back to armv6.\n");
+  return kArmv6;
+}
+
+// Get the CPU features enabled by the build.
+// For cross compilation the preprocessor symbols such as
+// CAN_USE_ARMV7_INSTRUCTIONS and CAN_USE_VFP3_INSTRUCTIONS can be used to
+// enable ARMv7 and VFPv3 instructions when building the snapshot. However,
+// these flags should be consistent with a supported ARM configuration:
+//  "armv6":       ARMv6 + VFPv2
+//  "armv7":       ARMv7 + VFPv3-D32 + NEON
+//  "armv7+sudiv": ARMv7 + VFPv4-D32 + NEON + SUDIV
+//  "armv8":       ARMv8 (+ all of the above)
+static constexpr unsigned CpuFeaturesFromCompiler() {
+  // TODO(jbramley): Once the build flags are simplified, these tests should
+  // also be simplified.
+
+  // Find compiler-implied features.
+#if defined(CAN_USE_ARMV8_INSTRUCTIONS) && \
+    defined(CAN_USE_ARMV7_INSTRUCTIONS) && \
+    defined(CAN_USE_SUDIV) && \
+    defined(CAN_USE_NEON) && \
+    defined(CAN_USE_VFP3_INSTRUCTIONS)
+  return kArmv8;
+#elif defined(CAN_USE_ARMV7_INSTRUCTIONS) && \
+      defined(CAN_USE_SUDIV) && \
+      defined(CAN_USE_NEON) && \
+      defined(CAN_USE_VFP3_INSTRUCTIONS)
+  return kArmv7WithSudiv;
+#elif defined(CAN_USE_ARMV7_INSTRUCTIONS) && \
+      defined(CAN_USE_NEON) && \
+      defined(CAN_USE_VFP3_INSTRUCTIONS)
+  return kArmv7;
+#else

[Michael Achenbach, 2016/08/12 10:16:37]

Same comment as flag-definitions.h - maybe raise error for wrong configs. Or
ignore if a clean up and simplification is coming in future CLs.

+  return kArmv6;
+#endif
 }
 
 
 void CpuFeatures::ProbeImpl(bool cross_compile) {
-  supported_ |= CpuFeaturesImpliedByCompiler();
   dcache_line_size_ = 64;
 
+  unsigned command_line = CpuFeaturesFromCommandLine();
   // Only use statically determined features for cross compile (snapshot).
-  if (cross_compile) return;
+  if (cross_compile) {
+    supported_ |= command_line & CpuFeaturesFromCompiler();
+    return;
+  }
 
 #ifndef __arm__
   // For the simulator build, use whatever the flags specify.
-  if (FLAG_enable_armv8) {
-    supported_ |= 1u << ARMv8;
-    // ARMv8 always features VFP and NEON.
-    supported_ |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
-    supported_ |= 1u << SUDIV;
-    if (FLAG_enable_movw_movt) supported_ |= 1u << MOVW_MOVT_IMMEDIATE_LOADS;
-  }
-  if (FLAG_enable_armv7) {
-    supported_ |= 1u << ARMv7;
-    if (FLAG_enable_vfp3) supported_ |= 1u << VFP3;
-    if (FLAG_enable_neon) supported_ |= 1u << NEON | 1u << VFP32DREGS;
-    if (FLAG_enable_sudiv) supported_ |= 1u << SUDIV;
-    if (FLAG_enable_movw_movt) supported_ |= 1u << MOVW_MOVT_IMMEDIATE_LOADS;
-    if (FLAG_enable_32dregs) supported_ |= 1u << VFP32DREGS;
+  supported_ |= command_line;
+
+  if (FLAG_enable_movw_movt && ((supported_ & kArmv7) == kArmv7)) {
+    supported_ |= 1u << MOVW_MOVT_IMMEDIATE_LOADS;
   }
 
 #else  // __arm__
   // Probe for additional features at runtime.
   base::CPU cpu;
-  if (FLAG_enable_vfp3 && cpu.has_vfp3()) {
-    // This implementation also sets the VFP flags if runtime
-    // detection of VFP returns true. VFPv3 implies ARMv7, see ARM DDI
-    // 0406B, page A1-6.
-    supported_ |= 1u << VFP3 | 1u << ARMv7;
+  // Runtime detection is slightly fuzzy, and some inferences are necessary.
+  unsigned runtime = kArmv6;
+  // NEON and VFPv3 imply at least ARMv7-A.
+  if (cpu.has_neon() && cpu.has_vfp3_d32()) {
+    DCHECK(cpu.has_vfp3());
+    runtime |= kArmv7;
+    if (cpu.has_idiva()) {
+      runtime |= kArmv7WithSudiv;

[Michael Achenbach, 2016/08/12 10:16:37]

Maybe nest the armv8 case within cpu.has_idiva()? Right now it looks like it
could be possible to get cpu.has_idiva() == false, but cpu.architecture() >= 8,
but it shouldn't be possible, right?

+    }
+    if (cpu.architecture() >= 8) {
+      runtime |= kArmv8;
+    }
   }
 
-  if (FLAG_enable_neon && cpu.has_neon()) supported_ |= 1u << NEON;
-  if (FLAG_enable_sudiv && cpu.has_idiva()) supported_ |= 1u << SUDIV;
+  // Use the best of the features found by CPU detection and those inferred from
+  // the build system. In both cases, restrict available features using the
+  // command-line. Note that the command-line flags are very permissive (kArmv8)
+  // by default.
+  supported_ |= command_line & CpuFeaturesFromCompiler();
+  supported_ |= command_line & runtime;
 
-  if (cpu.architecture() >= 7) {
-    if (FLAG_enable_armv7) supported_ |= 1u << ARMv7;
-    if (FLAG_enable_armv8 && cpu.architecture() >= 8) {
-      supported_ |= 1u << ARMv8;
-    }
-    // Use movw/movt for QUALCOMM ARMv7 cores.
-    if (FLAG_enable_movw_movt && cpu.implementer() == base::CPU::QUALCOMM) {
-      supported_ |= 1u << MOVW_MOVT_IMMEDIATE_LOADS;
-    }
+  // Additional tuning options.
+
+  // Prefer to use movw/movt for QUALCOMM ARMv7 cores.
+  if (FLAG_enable_movw_movt && ((supported_ & kArmv7) == kArmv7) &&
+      (cpu.implementer() == base::CPU::QUALCOMM)) {
+    supported_ |= 1u << MOVW_MOVT_IMMEDIATE_LOADS;
   }
 
   // ARM Cortex-A9 and Cortex-A5 have 32 byte cachelines.
@@ -132,11 +164,11 @@ void CpuFeatures::ProbeImpl(bool cross_compile) {
        cpu.part() == base::CPU::ARM_CORTEX_A9)) {
     dcache_line_size_ = 32;
   }
-
-  if (FLAG_enable_32dregs && cpu.has_vfp3_d32()) supported_ |= 1u << VFP32DREGS;
 #endif
 
-  DCHECK(!IsSupported(VFP3) || IsSupported(ARMv7));
+  DCHECK_IMPLIES(IsSupported(ARMv7_SUDIV), IsSupported(ARMv7));
+  DCHECK_IMPLIES(IsSupported(ARMv8), IsSupported(ARMv7_SUDIV));
+  DCHECK_IMPLIES(IsSupported(MOVW_MOVT_IMMEDIATE_LOADS), IsSupported(ARMv7));
 }
 
 
@@ -209,7 +241,7 @@ void CpuFeatures::PrintFeatures() {
 #else
   bool eabi_hardfloat = false;
 #endif
-    printf(" USE_EABI_HARDFLOAT=%d\n", eabi_hardfloat);
+  printf(" USE_EABI_HARDFLOAT=%d\n", eabi_hardfloat);
 }