[CodeGen] Refactor register operand parsing in MI parser (#181748)

- Refactor register operand parsing to eliminate duplicated LLT parsing
code.
- Additionally, fix the register operand syntax in MI LangRef to match
what the parser supports.

llvm/docs/MIRLangRef.rst

@@ -523,7 +523,7 @@ The full syntax of a register operand is shown below:
 
 .. code-block:: text
 
-    [<flags>] <register> [ :<subregister-idx-name> ] [ (tied-def <tied-op>) ]
+    [<flags>] <register> [ .<subregister-idx-name> ] [ :<register-class> ] [ (tied-def <tied-op>) ] [ (<type>) ]
 
 This example shows an instance of the X86 ``XOR32rr`` instruction that has
 5 register operands with different register flags:
@@ -532,6 +532,9 @@ This example shows an instance of the X86 ``XOR32rr`` instruction that has
 
   dead $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $al
 
+Note that subregister-index, register-class and type cannot be specified for
+physical registers. Additionally, tied-def can only be specified for a use.
+
 .. _register-flags:
 
 Register Flags
@@ -602,7 +605,7 @@ lower bits from the 32-bit virtual register 0 to the 8-bit virtual register 1:
 
 .. code-block:: text
 
-    %1 = COPY %0:sub_8bit
+    %1 = COPY %0.sub_8bit
 
 The names of the subregister indices are target specific, and are typically
 defined in the target's ``*RegisterInfo.td`` file.

llvm/lib/CodeGen/MIRParser/MIParser.cpp

@@ -1724,16 +1724,14 @@ bool MIParser::parseSubRegisterIndex(unsigned &SubReg) {
 }
 
 bool MIParser::parseRegisterTiedDefIndex(unsigned &TiedDefIdx) {
-  if (!consumeIfPresent(MIToken::kw_tied_def))
-    return true;
+  assert(Token.is(MIToken::kw_tied_def));
+  lex();
   if (Token.isNot(MIToken::IntegerLiteral))
     return error("expected an integer literal after 'tied-def'");
   if (getUnsigned(TiedDefIdx))
     return true;
   lex();
-  if (expectAndConsume(MIToken::rparen))
-    return true;
-  return false;
+  return expectAndConsume(MIToken::rparen);
 }
 
 bool MIParser::assignRegisterTies(MachineInstr &MI,
@@ -1781,6 +1779,8 @@ bool MIParser::parseRegisterOperand(MachineOperand &Dest,
     if (parseRegisterFlag(Flags))
       return true;
   }
+  // Update IsDef as we may have read a def flag.
+  IsDef = hasRegState(Flags, RegState::Define);
   if (!Token.isRegister())
     return error("expected a register after register flags");
   Register Reg;
@@ -1802,56 +1802,46 @@ bool MIParser::parseRegisterOperand(MachineOperand &Dest,
     if (parseRegisterClassOrBank(*RegInfo))
         return true;
   }
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  if (!hasRegState(Flags, RegState::Define)) {
-    if (consumeIfPresent(MIToken::lparen)) {
+
+  if (consumeIfPresent(MIToken::lparen)) {
+    // For a def, we only expect a type. For use we expect either a type or a
+    // tied-def. Additionally, for physical registers, we don't expect a type.
+    if (Token.is(MIToken::kw_tied_def)) {
+      if (IsDef)
+        return error("tied-def not supported for defs");
       unsigned Idx;
-      if (!parseRegisterTiedDefIndex(Idx))
-        TiedDefIdx = Idx;
-      else {
-        // Try a redundant low-level type.
-        LLT Ty;
-        if (parseLowLevelType(Token.location(), Ty))
-          return error("expected tied-def or low-level type after '('");
+      if (parseRegisterTiedDefIndex(Idx))
+        return true;
+      TiedDefIdx = Idx;
+    } else {
+      if (!Reg.isVirtual())
+        return error("unexpected type on physical register");
 
-        if (expectAndConsume(MIToken::rparen))
-          return true;
+      LLT Ty;
+      // If type parsing fails, forwad the parse error for defs.
+      if (parseLowLevelType(Token.location(), Ty))
+        return IsDef ? true
+                     : error("expected tied-def or low-level type after '('");
 
-        if (MRI.getType(Reg).isValid() && MRI.getType(Reg) != Ty)
-          return error("inconsistent type for generic virtual register");
+      if (expectAndConsume(MIToken::rparen))
+        return true;
 
-        MRI.setRegClassOrRegBank(Reg, static_cast<RegisterBank *>(nullptr));
-        MRI.setType(Reg, Ty);
-        MRI.noteNewVirtualRegister(Reg);
-      }
+      MachineRegisterInfo &MRI = MF.getRegInfo();
+      if (MRI.getType(Reg).isValid() && MRI.getType(Reg) != Ty)
+        return error("inconsistent type for generic virtual register");
+
+      MRI.setRegClassOrRegBank(Reg, static_cast<RegisterBank *>(nullptr));
+      MRI.setType(Reg, Ty);
+      MRI.noteNewVirtualRegister(Reg);
     }
-  } else if (consumeIfPresent(MIToken::lparen)) {
-    // Virtual registers may have a tpe with GlobalISel.
-    if (!Reg.isVirtual())
-      return error("unexpected type on physical register");
-
-    LLT Ty;
-    if (parseLowLevelType(Token.location(), Ty))
-      return true;
-
-    if (expectAndConsume(MIToken::rparen))
-      return true;
-
-    if (MRI.getType(Reg).isValid() && MRI.getType(Reg) != Ty)
-      return error("inconsistent type for generic virtual register");
-
-    MRI.setRegClassOrRegBank(Reg, static_cast<RegisterBank *>(nullptr));
-    MRI.setType(Reg, Ty);
-  } else if (Reg.isVirtual()) {
-    // Generic virtual registers must have a type.
-    // If we end up here this means the type hasn't been specified and
-    // this is bad!
+  } else if (IsDef && Reg.isVirtual()) {
+    // Generic virtual registers defs must have a type.
     if (RegInfo->Kind == VRegInfo::GENERIC ||
         RegInfo->Kind == VRegInfo::REGBANK)
       return error("generic virtual registers must have a type");
   }
 
-  if (hasRegState(Flags, RegState::Define)) {
+  if (IsDef) {
     if (hasRegState(Flags, RegState::Kill))
       return error("cannot have a killed def operand");
   } else {
@@ -1859,15 +1849,14 @@ bool MIParser::parseRegisterOperand(MachineOperand &Dest,
       return error("cannot have a dead use operand");
   }
 
-  Dest = MachineOperand::CreateReg(Reg, hasRegState(Flags, RegState::Define),
-                                   hasRegState(Flags, RegState::Implicit),
-                                   hasRegState(Flags, RegState::Kill),
-                                   hasRegState(Flags, RegState::Dead),
-                                   hasRegState(Flags, RegState::Undef),
-                                   hasRegState(Flags, RegState::EarlyClobber),
-                                   SubReg, hasRegState(Flags, RegState::Debug),
-                                   hasRegState(Flags, RegState::InternalRead),
-                                   hasRegState(Flags, RegState::Renamable));
+  Dest = MachineOperand::CreateReg(
+      Reg, IsDef, hasRegState(Flags, RegState::Implicit),
+      hasRegState(Flags, RegState::Kill), hasRegState(Flags, RegState::Dead),
+      hasRegState(Flags, RegState::Undef),
+      hasRegState(Flags, RegState::EarlyClobber), SubReg,
+      hasRegState(Flags, RegState::Debug),
+      hasRegState(Flags, RegState::InternalRead),
+      hasRegState(Flags, RegState::Renamable));
 
   return false;
 }

llvm/test/CodeGen/MIR/X86/expected-integer-after-tied-def.mir

@@ -17,7 +17,7 @@ body: |
   bb.0.entry:
     liveins: $rdi
 
-  ; CHECK: [[@LINE+1]]:78: expected tied-def or low-level type after '('
+  ; CHECK: [[@LINE+1]]:78: expected an integer literal after 'tied-def'
     INLINEASM &"$foo", 1, 2818058, def $rdi, 2147483657, killed $rdi(tied-def)
     $rax = COPY killed $rdi
     RET64 killed $rax

llvm/test/CodeGen/MIR/X86/invalid-tied-physical-reg-def.mir

@@ -0,0 +1,15 @@
+# RUN: not llc -mtriple=x86_64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+---
+name:            test
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi' }
+body: |
+  bb.0.entry:
+    liveins: $rdi
+
+  ; CHECK: [[@LINE+1]]:45: tied-def not supported for defs
+    INLINEASM &"$foo", 1, 2818058, def $rdi(tied-def 5), 2147483657, killed $rdi(tied-def 3)
+    $rax = COPY killed $rdi
+    RET64 killed $rax
+...

llvm/test/CodeGen/MIR/X86/invalid-type-physical-reg.mir

@@ -17,7 +17,7 @@ body: |
   bb.0.entry:
     liveins: $rdi
 
-  ; CHECK: [[@LINE+1]]:70: expected tied-def or low-level type after '('
+  ; CHECK: [[@LINE+1]]:70: unexpected type on physical register
     INLINEASM &"$foo", 1, 2818058, def $rdi, 2147483657, killed $rdi(3)
     $rax = COPY killed $rdi
     RET64 killed $rax