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

[Saleem Abdulrasool]

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1837
@@ +1836,3 @@
+    auto Entry = RegisterReqs.find(Name.lower());
+    // If no match, return failure.
+    if (Entry == RegisterReqs.end())
----------------
Seems like a superfluous comment.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1887
@@ +1886,3 @@
+
+  if (RegNum == 0) {
+    // Check for aliases registered via .req. Canonicalize to lower case.
----------------
This feels really really similar to the previous handling for the lookup.  Why not take an extra parameter to indicate if you want a GPR or a VFP register alias, and then share the code for this?

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3075
@@ -3035,1 +3074,3 @@
 
+  // First check for the AARCH64M-specific .req directive.
+  if (Parser.getTok().is(AsmToken::Identifier) &&
----------------
AArch64-M?  Why does this not apply to -A class CPUs?  I dont see where you are filtering them out.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3968
@@ +3967,3 @@
+AArch64AsmParser::parseDirectiveReq(StringRef Name, SMLoc L) {
+  Parser.Lex(); // Eat the '.req' token.
+  StringRef Kind;
----------------
This ordering feels weird.  You loose the ability to reference the mnemonic by lexing the token up front.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3969
@@ +3968,3 @@
+  Parser.Lex(); // Eat the '.req' token.
+  StringRef Kind;
+  SMLoc SRegLoc = getLoc();
----------------
This can be scoped to the RegNum == -1 case cant it?

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3998
@@ +3997,3 @@
+
+  auto pair = std::pair<bool, unsigned>(IsVector, RegNum);
+  if (RegisterReqs.GetOrCreateValue(Name, pair).getValue() != pair)
----------------
std::make_pair would be nicer.

================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:4011
@@ +4010,3 @@
+    Parser.eatToEndOfStatement();
+    Error(L, "unexpected input in .unreq directive.");
+    return false;
----------------
I think it would be better if the caret pointed to the invalid argument.

================
Comment at: test/MC/AArch64/dot-req-case-insensitive.s:1
@@ +1,2 @@
+// RUN: llvm-mc -triple=arm64 < %s | FileCheck %s
+_foo:
----------------
Please be more precise with the target.  -triple arm64-eabi is a good choice.

================
Comment at: test/MC/AArch64/dot-req-diagnostics.s:1
@@ +1,2 @@
+// RUN: not llvm-mc -triple aarch64-none-linux-gnu < %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ERROR --check-prefix=CHECK-ERROR-ARM64 < %t %s
----------------
Why the tempfile usage here?  You should be able to just use pipes for this.

================
Comment at: test/MC/AArch64/dot-req-diagnostics.s:2
@@ +1,3 @@
+// RUN: not llvm-mc -triple aarch64-none-linux-gnu < %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ERROR --check-prefix=CHECK-ERROR-ARM64 < %t %s
+
----------------
You arent using the CHECK-ERROR-ARM64 prefix.

================
Comment at: test/MC/AArch64/dot-req-diagnostics.s:7
@@ +6,3 @@
+fred .req x6
+        mov  x1, fred
+ada  .req v2.8b
----------------
Why the weird indentation?

================
Comment at: test/MC/AArch64/dot-req-diagnostics.s:13
@@ +12,3 @@
+// CHECK: mov x1, x5
+// CHECK-NOT: mov x1, x6
+// CHECK-ERROR: warning: ignoring redefinition of register alias 'fred'
----------------
These are not checked.  You need to do:

    not llvm-mc -triple aarch64-eabi %s 2>&1 | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ERROR

If you want to check the CHECK and CHECK-NOT lines.

================
Comment at: test/MC/AArch64/dot-req.s:12
@@ +11,3 @@
+ada .req w1
+        mov ada, bob
+.unreq bob
----------------
What happened to Alice?  I thought she always accompanied Bob :-p.

http://reviews.llvm.org/D4206