[PATCH] [AArch64] Refactor AArch64NamedImmMapper to become dependent on subtarget features.
James Molloy
james.molloy at arm.com
Thu Apr 16 03:21:05 PDT 2015
Hi Vlad,
Looks like a mostly mechanical change. I don't like one aspect of it, but if you change that you can commit.
Cheers,
James
REPOSITORY
rL LLVM
================
Comment at: lib/Target/AArch64/Utils/AArch64BaseInfo.h:285
@@ +284,3 @@
+ // empty AvailableForFeatures means "always-on"
+ template <typename T> bool isEqual(T Other) const;
+ template <typename T> bool isEqual(T Other, uint64_t FeatureBits) const {
----------------
I think this is overly convoluted for such a simple function. The same function name does different things depending on the overload (one checks Name and the other Value). I don't think this is intuitive.
Just simply:
bool isNameEqual(std::string Other, uint64_t FeatureBits=~0ULL) {
if (AvailableForFeatures && !(AvailableForFeatures & FeatureBits))
return false;
return Name == Other;
}
And similarly for Value, would be more clear.
http://reviews.llvm.org/D8496
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list