[PATCH] [AArch64] Refactor AArch64NamedImmMapper to become dependent on subtarget features.

[Eric Christopher]

Some comments inline.

-eric


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/AArch64/Utils/AArch64BaseInfo.h:283
@@ -282,1 +282,3 @@
     uint32_t Value;
+    uint64_t AvailableForFeatures;
+    // empty AvailableForFeatures means "always-on"
----------------
This is a pretty terrible name. What do you mean here?

================
Comment at: lib/Target/AArch64/Utils/AArch64BaseInfo.h:284
@@ +283,3 @@
+    uint64_t AvailableForFeatures;
+    // empty AvailableForFeatures means "always-on"
+    template <typename T> bool isEqual(T Other) const;
----------------
Comments are full sentences.

================
Comment at: lib/Target/AArch64/Utils/AArch64BaseInfo.h:286
@@ +285,3 @@
+    template <typename T> bool isEqual(T Other) const;
+    template <typename T> bool isEqual(T Other, uint64_t FeatureBits) const {
+      if (AvailableForFeatures && !(AvailableForFeatures & FeatureBits))
----------------
I'm not a fan of this interface name. isEqual taking features and not changing the name isn't very descriptive.

================
Comment at: lib/Target/AArch64/Utils/AArch64BaseInfo.h:297
@@ -288,3 +296,3 @@
 
-  StringRef toString(uint32_t Value, bool &Valid) const;
-  uint32_t fromString(StringRef Name, bool &Valid) const;
+  StringRef toString(uint32_t Value, uint64_t FeatureBits, bool &Valid) const;
+  uint32_t fromString(StringRef Name, uint64_t FeatureBits, bool &Valid) const;
----------------
If you're taking FeatureBits now this should probably change names to make the interface more clear.

Also (and I realize this was already like this) the functions need a comment as to what they're for.

http://reviews.llvm.org/D8496

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/