commit fdf332ab24163073b964d8017435332984d539f2
author Jacob Bramley <jacob.bramley@arm.com> Mon Sep 17 11:17:54 2018 +0100
committer Jacob Bramley <jacob.bramley@arm.com> Wed Sep 26 10:21:03 2018 +0000
tree 1471ca09bd213d32045eb5abbe26e8b8aa4b566c
parent 7317f8b590cf391c60f53891bab7bff85fc36bef

Give CPUFeatures::kNone a stable value.

This makes the simulation pseudo-instruction mechanism a bit more stable across
VIXL versions.

We still don't guarantee that simulation pseusdo-instructions generated with the
MacroAssembler will work with a Simulator from a different version of VIXL, but
this patch has no real cost for VIXL, and might make things easier for someone.

Change-Id: Idd92328801a88f54a8d30db86304746a22ccb1df

src/aarch64/simulator-aarch64.cc

diff --git a/src/aarch64/simulator-aarch64.cc b/src/aarch64/simulator-aarch64.cc
index c09650d..4763a54 100644
--- a/src/aarch64/simulator-aarch64.cc
+++ b/src/aarch64/simulator-aarch64.cc
@@ -6597,7 +6597,7 @@
   VIXL_ASSERT(instr->Mask(ExceptionMask) == HLT);
 
   typedef ConfigureCPUFeaturesElementType ElementType;
-  VIXL_ASSERT(CPUFeatures::kNumberOfFeatures <=
+  VIXL_ASSERT(CPUFeatures::kNumberOfFeatures <
               std::numeric_limits<ElementType>::max());
 
   // k{Set,Enable,Disable}CPUFeatures have the same parameter encoding.
@@ -6610,7 +6610,7 @@
   while (true) {
     ElementType feature = Memory::Read<ElementType>(instr + offset);
     offset += element_size;
-    if (feature == CPUFeatures::kNone) break;
+    if (feature == static_cast<ElementType>(CPUFeatures::kNone)) break;
     parameters.Combine(static_cast<CPUFeatures::Feature>(feature));
   }
 

src/cpu-features.cc

diff --git a/src/cpu-features.cc b/src/cpu-features.cc
index 8edd1ff..c366670 100644
--- a/src/cpu-features.cc
+++ b/src/cpu-features.cc
@@ -148,6 +148,8 @@
 #undef VIXL_FORMAT_FEATURE
     case CPUFeatures::kNone:
       return os << "none";
+    case CPUFeatures::kNumberOfFeatures:
+      VIXL_UNREACHABLE();
   }
   // clang-format on
   VIXL_UNREACHABLE();
@@ -187,12 +189,13 @@
   VIXL_ASSERT(IsValid());
   do {
     // Find the next feature. The order is unspecified.
-    VIXL_STATIC_ASSERT(CPUFeatures::kNone == CPUFeatures::kNumberOfFeatures);
-    if (feature_ == CPUFeatures::kNone) {
-      feature_ = static_cast<CPUFeatures::Feature>(0);
-    } else {
-      feature_ = static_cast<CPUFeatures::Feature>(feature_ + 1);
+    feature_ = static_cast<CPUFeatures::Feature>(feature_ + 1);
+    if (feature_ == CPUFeatures::kNumberOfFeatures) {
+      feature_ = CPUFeatures::kNone;
+      VIXL_STATIC_ASSERT(CPUFeatures::kNone == -1);
     }
+    VIXL_ASSERT(CPUFeatures::kNone <= feature_);
+    VIXL_ASSERT(feature_ < CPUFeatures::kNumberOfFeatures);
     // cpu_features_->Has(kNone) is always true, so this will terminate even if
     // the features list is empty.
   } while (!cpu_features_->Has(feature_));

src/cpu-features.h

diff --git a/src/cpu-features.h b/src/cpu-features.h
index 853421b..f94b955 100644
--- a/src/cpu-features.h
+++ b/src/cpu-features.h
@@ -167,11 +167,11 @@
     // Refer to VIXL_CPU_FEATURE_LIST (above) for the list of feature names that
     // this class supports.
 
+    kNone = -1,
 #define VIXL_DECLARE_FEATURE(SYMBOL, NAME, CPUINFO) SYMBOL,
     VIXL_CPU_FEATURE_LIST(VIXL_DECLARE_FEATURE)
 #undef VIXL_DECLARE_FEATURE
-    kNone,
-    kNumberOfFeatures = kNone
+    kNumberOfFeatures
   };
   // clang-format on