aarch64: support target-specific .req assembler directive

[jannau]

Summary

Based on the support for .req on ARM. The aarch64 variant has to keep track if the alias register was a vector register (v0-31) or a general purpose or VFP/Advanced SIMD ([bhsdq]0-31) register.

[compnerd]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1837

Seems like a superfluous comment.

1887

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?

3075

AArch64-M? Why does this not apply to -A class CPUs? I dont see where you are filtering them out.

3968

This ordering feels weird. You loose the ability to reference the mnemonic by lexing the token up front.

3969

This can be scoped to the RegNum == -1 case cant it?

3998

std::make_pair would be nicer.

4011

I think it would be better if the caret pointed to the invalid argument.

test/MC/AArch64/dot-req-case-insensitive.s

2

Please be more precise with the target. -triple arm64-eabi is a good choice.

test/MC/AArch64/dot-req-diagnostics.s

2

Why the tempfile usage here? You should be able to just use pipes for this.

3

You arent using the CHECK-ERROR-ARM64 prefix.

8

Why the weird indentation?

14

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.

test/MC/AArch64/dot-req.s

13

What happened to Alice? I thought she always accompanied Bob :-p.

[jannau]

All review comments not individually commented on should be addressed

[compnerd]

LGTM with some minor nits.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

57

This should be matchRegisterNameAlias ... we changed the style guide at some point to camel case for function names.

1830

Extraneous space.

1833

Almost feels that this might be better as a ternary.

3977

C++ style casts are preferable IMO.

4001

Return type goes on the same line unless it doesn't fit.

[jannau]

addressed all review comments

[jannau]

rebased on top of r211605 - "Implement pseudo LDR <reg>, =<literal/label> for AArch64"

[jannau]

removed tabs used for indent crept in due editor misconfiguration

[compnerd]

Committed as SVN r212161. Thanks for the patch!

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

   MCSubtargetInfo &STI;
   MCAsmParser &Parser;
 
+  // Map of register aliases registers via the .req directive.
+  StringMap<std::pair<bool, unsigned> > RegisterReqs;
+
   AArch64TargetStreamer &getTargetStreamer() {
     MCTargetStreamer &TS = *getParser().getStreamer().getTargetStreamer();
     return static_cast<AArch64TargetStreamer &>(TS);

   bool parseSysAlias(StringRef Name, SMLoc NameLoc, OperandVector &Operands);
   AArch64CC::CondCode parseCondCodeString(StringRef Cond);
   bool parseCondCode(OperandVector &Operands, bool invertCondCode);
+  unsigned matchRegisterNameAlias(StringRef Name, bool isVector);
   int tryParseRegister();
   int tryMatchVectorRegister(StringRef &Kind, bool expected);
   bool parseRegister(OperandVector &Operands);

   bool parseDirectiveLOH(StringRef LOH, SMLoc L);
   bool parseDirectiveLtorg(SMLoc L);
 
+  bool parseDirectiveReq(StringRef Name, SMLoc L);
+  bool parseDirectiveUnreq(SMLoc L);
+
   bool validateInstruction(MCInst &Inst, SmallVectorImpl<SMLoc> &Loc);
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,

   return (RegNo == (unsigned)-1);
 }
 
+// Matches a register name or register alias previously defined by '.req'
+unsigned AArch64AsmParser::matchRegisterNameAlias(StringRef Name,
+                                                  bool isVector) {
+  unsigned RegNum = isVector ? matchVectorRegName(Name)
+                             : MatchRegisterName(Name);
+
+  if (RegNum == 0) {
+    // Check for aliases registered via .req. Canonicalize to lower case.
+    // That's more consistent since register names are case insensitive, and
+    // it's how the original entry was passed in from MC/MCParser/AsmParser.
+    auto Entry = RegisterReqs.find(Name.lower());
+    if (Entry == RegisterReqs.end())
+      return 0;
+    // set RegNum if the match is the right kind of register
+    if (isVector == Entry->getValue().first)
+      RegNum = Entry->getValue().second;
+  }
+  return RegNum;
+}
+
 /// tryParseRegister - Try to parse a register name. The token must be an
 /// Identifier when called, and if it is a register name the token is eaten and
 /// the register is added to the operand list.

   assert(Tok.is(AsmToken::Identifier) && "Token is not an Identifier");
 
   std::string lowerCase = Tok.getString().lower();
-  unsigned RegNum = MatchRegisterName(lowerCase);
+  unsigned RegNum = matchRegisterNameAlias(lowerCase, false);
   // Also handle a few aliases of registers.
   if (RegNum == 0)
     RegNum = StringSwitch<unsigned>(lowerCase)

   // a '.'.
   size_t Start = 0, Next = Name.find('.');
   StringRef Head = Name.slice(Start, Next);
-  unsigned RegNum = matchVectorRegName(Head);
+  unsigned RegNum = matchRegisterNameAlias(Head, true);
+
   if (RegNum) {
     if (Next != StringRef::npos) {
       Kind = Name.slice(Next, StringRef::npos);

   if (!Tok.is(AsmToken::Identifier))
     return MatchOperand_NoMatch;
 
-  unsigned RegNum = MatchRegisterName(Tok.getString().lower());
+  unsigned RegNum = matchRegisterNameAlias(Tok.getString().lower(), false);
 
   MCContext &Ctx = getContext();
   const MCRegisterInfo *RI = Ctx.getRegisterInfo();

              .Case("bnv", "b.nv")
              .Default(Name);
 
+  // First check for the AArch64-specific .req directive.
+  if (Parser.getTok().is(AsmToken::Identifier) &&
+      Parser.getTok().getIdentifier() == ".req") {
+    parseDirectiveReq(Name, NameLoc);
+    // We always return 'error' for this, as we're done with this
+    // statement and don't need to match the 'instruction."
+    return true;
+  }
+
   // Create the leading tokens for the mnemonic, split by '.' characters.
   size_t Start = 0, Next = Name.find('.');
   StringRef Head = Name.slice(Start, Next);

     return parseDirectiveTLSDescCall(Loc);
   if (IDVal == ".ltorg" || IDVal == ".pool")
     return parseDirectiveLtorg(Loc);
+  if (IDVal == ".unreq")
+    return parseDirectiveUnreq(DirectiveID.getLoc());
+
   return parseDirectiveLOH(IDVal, Loc);
 }
 

   return false;
 }
 
+/// parseDirectiveReq
+///  ::= name .req registername
+bool AArch64AsmParser::parseDirectiveReq(StringRef Name, SMLoc L) {
+  Parser.Lex(); // Eat the '.req' token.
+  SMLoc SRegLoc = getLoc();
+  unsigned RegNum = tryParseRegister();
+  bool IsVector = false;
+
+  if (RegNum == static_cast<unsigned>(-1)) {
+    StringRef Kind;
+    RegNum = tryMatchVectorRegister(Kind, false);
+    if (!Kind.empty()) {
+      Error(SRegLoc, "vector register without type specifier expected");
+      return false;
+    }
+    IsVector = true;
+  }
+
+  if (RegNum == static_cast<unsigned>(-1)) {
+    Parser.eatToEndOfStatement();
+    Error(SRegLoc, "register name or alias expected");
+    return false;
+  }
+
+  // Shouldn't be anything else.
+  if (Parser.getTok().isNot(AsmToken::EndOfStatement)) {
+    Error(Parser.getTok().getLoc(), "unexpected input in .req directive");
+    Parser.eatToEndOfStatement();
+    return false;
+  }
+
+  Parser.Lex(); // Consume the EndOfStatement
+
+  auto pair = std::make_pair(IsVector, RegNum);
+  if (RegisterReqs.GetOrCreateValue(Name, pair).getValue() != pair)
+    Warning(L, "ignoring redefinition of register alias '" + Name + "'");
+
+  return true;
+}
+
+/// parseDirectiveUneq
+///  ::= .unreq registername
+bool AArch64AsmParser::parseDirectiveUnreq(SMLoc L) {
+  if (Parser.getTok().isNot(AsmToken::Identifier)) {
+    Error(Parser.getTok().getLoc(), "unexpected input in .unreq directive.");
+    Parser.eatToEndOfStatement();
+    return false;
+  }
+  RegisterReqs.erase(Parser.getTok().getIdentifier().lower());
+  Parser.Lex(); // Eat the identifier.
+  return false;
+}
+
 bool
 AArch64AsmParser::classifySymbolRef(const MCExpr *Expr,
                                     AArch64MCExpr::VariantKind &ELFRefKind,

test/MC/AArch64/dot-req-case-insensitive.s

+// RUN: llvm-mc -triple=arm64-eabi < %s | FileCheck %s
+_foo:
+        OBJECT .req x2
+        mov x4, OBJECT
+        mov x4, oBjEcT
+        .unreq oBJECT
+
+_foo2:
+        OBJECT .req w5
+        mov w4, OBJECT
+        .unreq OBJECT
+
+// CHECK-LABEL: _foo:
+// CHECK: mov x4, x2
+// CHECK: mov x4, x2
+
+// CHECK-LABEL: _foo2:
+// CHECK: mov w4, w5

test/MC/AArch64/dot-req-diagnostics.s

+// RUN: not llvm-mc -triple aarch64-none-linux-gnu < %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-ERROR %s
+
+bar:
+        fred .req x5
+        fred .req x6
+// CHECK-ERROR: warning: ignoring redefinition of register alias 'fred'
+// CHECK-ERROR: fred .req x6
+// CHECK-ERROR: ^
+
+        ada  .req v2.8b
+// CHECK-ERROR: error: vector register without type specifier expected
+// CHECK-ERROR: ada  .req v2.8b
+// CHECK-ERROR:           ^
+
+        bob  .req lisa
+// CHECK-ERROR: error: register name or alias expected
+// CHECK-ERROR: bob  .req lisa
+// CHECK-ERROR:           ^
+
+        lisa .req x1, 23
+// CHECK-ERROR: error: unexpected input in .req directive
+// CHECK-ERROR: lisa .req x1, 23
+// CHECK-ERROR:             ^
+
+        mov  bob, fred
+// CHECK-ERROR: error: invalid operand for instruction
+// CHECK-ERROR: mov  bob, fred
+// CHECK-ERROR:      ^
+
+        .unreq 1
+// CHECK-ERROR: error: unexpected input in .unreq directive.
+// CHECK-ERROR: .unreq 1
+// CHECK-ERROR:        ^
+
+        mov  x1, fred
+// CHECK: mov x1, x5
+// CHECK-NOT: mov x1, x6

test/MC/AArch64/dot-req.s

+// RUN: llvm-mc -triple=aarch64-none-linux-gnu -show-encoding < %s | FileCheck %s
+
+bar:
+        fred .req x5
+        mov fred, x11
+        .unreq fred
+        fred .req w6
+        mov w1, fred
+
+        bob .req fred
+        ada .req w1
+        mov ada, bob
+        .unreq bob
+        .unreq fred
+        .unreq ada
+// CHECK: mov      x5, x11                // encoding: [0xe5,0x03,0x0b,0xaa]
+// CHECK: mov      w1, w6                 // encoding: [0xe1,0x03,0x06,0x2a]
+// CHECK: mov      w1, w6                 // encoding: [0xe1,0x03,0x06,0x2a]
+
+        bob     .req b6
+        hanah   .req h5
+        sam     .req s4
+        dora    .req d3
+        quentin .req q2
+        vesna   .req v1
+        addv bob,     v0.8b
+        mov  hanah,   v4.h[3]
+        fadd s0,      sam,     sam
+        fmov d2,      dora
+        ldr  quentin, [sp]
+        mov  v0.8b,   vesna.8b
+// CHECK: addv    b6, v0.8b               // encoding: [0x06,0xb8,0x31,0x0e]
+// CHECK: mov     h5, v4.h[3]             // encoding: [0x85,0x04,0x0e,0x5e]
+// CHECK: fadd    s0, s4, s4              // encoding: [0x80,0x28,0x24,0x1e]
+// CHECK: fmov    d2, d3                  // encoding: [0x62,0x40,0x60,0x1e]
+// CHECK: ldr      q2, [sp]               // encoding: [0xe2,0x03,0xc0,0x3d]
+// CHECK: mov             v0.8b, v1.8b    // encoding: [0x20,0x1c,0xa1,0x0e]