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

James Molloy james.molloy at arm.com
Tue Mar 24 03:04:06 PDT 2015


Hi Vladimir,

Thanks for doing this. I think there's nothing massively contentious in your patch, but it's difficult to review because it's a merge of three different patches, as far as I can see:

1. Adding SubTargetFeatureBits everywhere.
2. Changing the AsmParser to take a string for barriers etc (this seems orthogonal to me)
3. Re-bikeshedding (Pair->Mapper, if/else -> ?:)

So please separate it out into three patches, each of which can be trivially reviewed.

Cheers,

James


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/AArch64/Utils/AArch64BaseInfo.h:305
@@ +304,3 @@
+  bool hasFeature(uint64_t SubTargetFeature) const {
+    return SubTargetFeature == 0 || (SubTargetFeature & FeatureBits) != 0;
+  }
----------------
Why "SubTargetFeature == 0"? This seems strange. I would have expected just:

    return SubTargetFeature & FeatureBits;

http://reviews.llvm.org/D8496

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






More information about the llvm-commits mailing list