[PATCH] [AArch64, ARM] Add v8.1a architecture and generic cpu

Tim Northover t.p.northover at gmail.com
Fri Mar 20 19:11:42 PDT 2015


This looks OK, with one small clarification. (Feel free to commit with that change unless you disagree).

Tim.


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:669
@@ -668,3 +668,3 @@
 
-  if (CPUString != "generic") {
+  if (CPUString.find("generic")) {
     // FIXME: remove krait check when GNU tools support krait cpu
----------------
I think this is a bit non-obvious. I grant that it's correct (and equivalent to !StringRef::startswith), but I think this looks too much like "if generic is in CPUString". 

I'd suggest an explicit comparison against 0, which might make people at least consider whether string::find returns an index and what it returns for failure.

http://reviews.llvm.org/D8505

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






More information about the llvm-commits mailing list