[PATCH] aarch64: support target-specific .req assembler directive

Saleem Abdulrasool compnerd at compnerd.org
Mon Jun 23 22:54:18 PDT 2014


LGTM with some minor nits.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:57
@@ -53,2 +56,3 @@
   bool parseCondCode(OperandVector &Operands, bool invertCondCode);
+  unsigned MatchRegisterNameAlias(StringRef Name, bool isVector);
   int tryParseRegister();
----------------
This should be `matchRegisterNameAlias` ... we changed the style guide at some point to camel case for function names.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1830
@@ +1829,3 @@
+						  bool isVector) {
+
+  unsigned RegNum;
----------------
Extraneous space.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1833
@@ +1832,3 @@
+
+  if (isVector)
+    RegNum = matchVectorRegName(Name);
----------------
Almost feels that this might be better as a ternary.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3977
@@ +3976,3 @@
+
+  if (RegNum == (unsigned)-1) {
+    Parser.eatToEndOfStatement();
----------------
C++ style casts are preferable IMO.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:4001
@@ +4000,3 @@
+///  ::= .unreq registername
+bool
+AArch64AsmParser::parseDirectiveUnreq(SMLoc L) {
----------------
Return type goes on the same line unless it doesn't fit.

http://reviews.llvm.org/D4206






More information about the llvm-commits mailing list