MIRParser: Allow register class specification on operand

[MatzeB]

Summary

You can now define the register class of a virtual register on the
operand itself avoiding the need to use a "registers:" block.

Example: "%0:gr64 = COPY %rax"

[MatzeB]

ping

[qcolombet]

lib/CodeGen/MIRParser/MIParser.cpp

138 ↗

(On Diff #64085)

parserRegisterClassOrBank?

851 ↗

(On Diff #64085)

We should look for register banks as well.

1010 ↗

(On Diff #64085)

I believe we should have the same syntax for register bank.

test/CodeGen/MIR/X86/register-operand-class.mir

12 ↗

(On Diff #64085)

We shouldn’t need to emit that part anymore, right?
Probably a follow-up patch, though.

[MatzeB]

Updated to handle regbanks.

[MatzeB]

test/CodeGen/MIR/X86/register-operand-class.mir

12 ↗

(On Diff #64085)

yep, that is for a followup and needs some policy decisions like what to do with virtual registers that aren't used anywhere.

[MatzeB]

ping

[qcolombet]

LGTM

test/CodeGen/MIR/X86/register-operand-class.mir

19 ↗

(On Diff #81808)

Just a thought, shouldn't we make the class/bank on the definition mandatory?

[MatzeB]

Thanks for the review.

test/CodeGen/MIR/X86/register-operand-class.mir

19 ↗

(On Diff #81808)

Yeah, enforcing this for every def (and function live-in declaration) would be a good thing. First however we need a patch that lets the MIRPrinter use the new syntax here.

llvm/trunk/lib/CodeGen/MIRParser/MIParser.h

struct VRegInfo {
   union {
     const TargetRegisterClass *RC;
     const RegisterBank *RegBank;
   } D;
   unsigned VReg;
   unsigned PreferredReg = 0;
 };
 
+typedef StringMap<const TargetRegisterClass*> Name2RegClassMap;
+typedef StringMap<const RegisterBank*> Name2RegBankMap;
+
 struct PerFunctionMIParsingState {
   BumpPtrAllocator Allocator;
   MachineFunction &MF;
   SourceMgr *SM;
   const SlotMapping &IRSlots;
+  const Name2RegClassMap &Names2RegClasses;
+  const Name2RegBankMap &Names2RegBanks;
 
   DenseMap<unsigned, MachineBasicBlock *> MBBSlots;
   DenseMap<unsigned, VRegInfo*> VRegInfos;
   DenseMap<unsigned, int> FixedStackObjectSlots;
   DenseMap<unsigned, int> StackObjectSlots;
   DenseMap<unsigned, unsigned> ConstantPoolSlots;
   DenseMap<unsigned, unsigned> JumpTableSlots;
 
   PerFunctionMIParsingState(MachineFunction &MF, SourceMgr &SM,
-                            const SlotMapping &IRSlots);
+                            const SlotMapping &IRSlots,
+                            const Name2RegClassMap &Names2RegClasses,
+                            const Name2RegBankMap &Names2RegBanks);
 
   VRegInfo &getVRegInfo(unsigned VReg);
 };
 
 /// Parse the machine basic block definitions, and skip the machine
 /// instructions.
 ///
 /// This function runs the first parsing pass on the machine function's body.

llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp

 #include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/Target/TargetIntrinsicInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cctype>
 
 using namespace llvm;
 
 PerFunctionMIParsingState::PerFunctionMIParsingState(MachineFunction &MF,
-    SourceMgr &SM, const SlotMapping &IRSlots)
-  : MF(MF), SM(&SM), IRSlots(IRSlots) {
+    SourceMgr &SM, const SlotMapping &IRSlots,
+    const Name2RegClassMap &Names2RegClasses,
+    const Name2RegBankMap &Names2RegBanks)
+  : MF(MF), SM(&SM), IRSlots(IRSlots), Names2RegClasses(Names2RegClasses),
+    Names2RegBanks(Names2RegBanks) {
 }
 
 VRegInfo &PerFunctionMIParsingState::getVRegInfo(unsigned Num) {
   auto I = VRegInfos.insert(std::make_pair(Num, nullptr));
   if (I.second) {
     MachineRegisterInfo &MRI = MF.getRegInfo();
     VRegInfo *Info = new (Allocator) VRegInfo;
     Info->VReg = MRI.createIncompleteVirtualRegister();
public:
   bool parseBasicBlock(MachineBasicBlock &MBB);
   bool parseBasicBlockLiveins(MachineBasicBlock &MBB);
   bool parseBasicBlockSuccessors(MachineBasicBlock &MBB);
 
   bool parseNamedRegister(unsigned &Reg);
   bool parseVirtualRegister(VRegInfo *&Info);
   bool parseRegister(unsigned &Reg, VRegInfo *&VRegInfo);
   bool parseRegisterFlag(unsigned &Flags);
+  bool parseRegisterClassOrBank(VRegInfo &RegInfo);
   bool parseSubRegisterIndex(unsigned &SubReg);
   bool parseRegisterTiedDefIndex(unsigned &TiedDefIdx);
   bool parseRegisterOperand(MachineOperand &Dest,
                             Optional<unsigned> &TiedDefIdx, bool IsDef = false);
   bool parseImmediateOperand(MachineOperand &Dest);
   bool parseIRConstant(StringRef::iterator Loc, StringRef Source,
                        const Constant *&C);
   bool parseIRConstant(StringRef::iterator Loc, const Constant *&C);
case MIToken::VirtualRegister:
     Reg = Info->VReg;
     return false;
   // TODO: Parse other register kinds.
   default:
     llvm_unreachable("The current token should be a register");
   }
 }
 
+bool MIParser::parseRegisterClassOrBank(VRegInfo &RegInfo) {
+  if (Token.isNot(MIToken::Identifier))
+    return error("expected a register class or register bank name");
+  StringRef::iterator Loc = Token.location();
+  StringRef Name = Token.stringValue();
+
+  // Was it a register class?
+  auto RCNameI = PFS.Names2RegClasses.find(Name);
+  if (RCNameI != PFS.Names2RegClasses.end()) {
+    lex();
+    const TargetRegisterClass &RC = *RCNameI->getValue();
+
+    switch (RegInfo.Kind) {
+    case VRegInfo::UNKNOWN:
+    case VRegInfo::NORMAL:
+      RegInfo.Kind = VRegInfo::NORMAL;
+      if (RegInfo.Explicit && RegInfo.D.RC != &RC) {
+        const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
+        return error(Loc, Twine("conflicting register classes, previously: ") +
+                     Twine(TRI.getRegClassName(RegInfo.D.RC)));
+      }
+      RegInfo.D.RC = &RC;
+      RegInfo.Explicit = true;
+      return false;
+
+    case VRegInfo::GENERIC:
+    case VRegInfo::REGBANK:
+      return error(Loc, "register class specification on generic register");
+    }
+    llvm_unreachable("Unexpected register kind");
+  }
+
+  // Should be a register bank.
+  auto RBNameI = PFS.Names2RegBanks.find(Name);
+  lex();
+  if (RBNameI == PFS.Names2RegBanks.end())
+    return error(Loc, "expected a register class or register bank name");
+
+  const RegisterBank &RegBank = *RBNameI->getValue();
+  switch (RegInfo.Kind) {
+  case VRegInfo::UNKNOWN:
+  case VRegInfo::GENERIC:
+  case VRegInfo::REGBANK:
+    RegInfo.Kind = VRegInfo::REGBANK;
+    if (RegInfo.Explicit && RegInfo.D.RegBank != &RegBank)
+      return error(Loc, "conflicting register banks");
+    RegInfo.D.RegBank = &RegBank;
+    RegInfo.Explicit = true;
+    return false;
+
+  case VRegInfo::NORMAL:
+    return error(Loc, "register class specification on normal register");
+  }
+  llvm_unreachable("Unexpected register kind");
+}
+
 bool MIParser::parseRegisterFlag(unsigned &Flags) {
   const unsigned OldFlags = Flags;
   switch (Token.kind()) {
   case MIToken::kw_implicit:
     Flags |= RegState::Implicit;
     break;
   case MIToken::kw_implicit_define:
     Flags |= RegState::ImplicitDefine;
bool MIParser::parseRegisterOperand(MachineOperand &Dest,
   lex();
   unsigned SubReg = 0;
   if (Token.is(MIToken::dot)) {
     if (parseSubRegisterIndex(SubReg))
       return true;
     if (!TargetRegisterInfo::isVirtualRegister(Reg))
       return error("subregister index expects a virtual register");
   }
+  if (Token.is(MIToken::colon)) {
+    if (!TargetRegisterInfo::isVirtualRegister(Reg))
+      return error("register class specification expects a virtual register");
+    lex();
+    if (parseRegisterClassOrBank(*RegInfo))
+        return true;
+  }
   MachineRegisterInfo &MRI = MF.getRegInfo();
   if ((Flags & RegState::Define) == 0) {
     if (consumeIfPresent(MIToken::lparen)) {
       unsigned Idx;
       if (!parseRegisterTiedDefIndex(Idx))
         TiedDefIdx = Idx;
       else {
         // Try a redundant low-level type.

llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp

 /// file.
 class MIRParserImpl {
   SourceMgr SM;
   StringRef Filename;
   LLVMContext &Context;
   StringMap<std::unique_ptr<yaml::MachineFunction>> Functions;
   SlotMapping IRSlots;
   /// Maps from register class names to register classes.
-  StringMap<const TargetRegisterClass *> Names2RegClasses;
+  Name2RegClassMap Names2RegClasses;
   /// Maps from register bank names to register banks.
-  StringMap<const RegisterBank *> Names2RegBanks;
+  Name2RegBankMap Names2RegBanks;
 
 public:
   MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents, StringRef Filename,
                 LLVMContext &Context);
 
   void reportDiagnostic(const SMDiagnostic &Diag);
 
   /// Report an error with the given message at unknown location.

 }
 
 bool MIRParserImpl::initializeMachineFunction(MachineFunction &MF) {
   auto It = Functions.find(MF.getName());
   if (It == Functions.end())
     return error(Twine("no machine function information for function '") +
                  MF.getName() + "' in the MIR file");
   // TODO: Recreate the machine function.
+  initNames2RegClasses(MF);
+  initNames2RegBanks(MF);
   const yaml::MachineFunction &YamlMF = *It->getValue();
   if (YamlMF.Alignment)
     MF.setAlignment(YamlMF.Alignment);
   MF.setExposesReturnsTwice(YamlMF.ExposesReturnsTwice);
 
   if (YamlMF.Legalized)
     MF.getProperties().set(MachineFunctionProperties::Property::Legalized);
   if (YamlMF.RegBankSelected)
     MF.getProperties().set(
         MachineFunctionProperties::Property::RegBankSelected);
   if (YamlMF.Selected)
     MF.getProperties().set(MachineFunctionProperties::Property::Selected);
 
-  PerFunctionMIParsingState PFS(MF, SM, IRSlots);
+  PerFunctionMIParsingState PFS(MF, SM, IRSlots, Names2RegClasses,
+                                Names2RegBanks);
   if (parseRegisterInfo(PFS, YamlMF))
     return true;
   if (!YamlMF.Constants.empty()) {
     auto *ConstantPool = MF.getConstantPool();
     assert(ConstantPool && "Constant pool must be created");
     if (initializeConstantPool(PFS, *ConstantPool, YamlMF))
       return true;
   }
for (unsigned I = 0, E = RBI->getNumRegBanks(); I < E; ++I) {
     const auto &RegBank = RBI->getRegBank(I);
     Names2RegBanks.insert(
         std::make_pair(StringRef(RegBank.getName()).lower(), &RegBank));
   }
 }
 
 const TargetRegisterClass *MIRParserImpl::getRegClass(const MachineFunction &MF,
                                                       StringRef Name) {
-  initNames2RegClasses(MF);
   auto RegClassInfo = Names2RegClasses.find(Name);
   if (RegClassInfo == Names2RegClasses.end())
     return nullptr;
   return RegClassInfo->getValue();
 }
 
 const RegisterBank *MIRParserImpl::getRegBank(const MachineFunction &MF,
                                               StringRef Name) {
-  initNames2RegBanks(MF);
   auto RegBankInfo = Names2RegBanks.find(Name);
   if (RegBankInfo == Names2RegBanks.end())
     return nullptr;
   return RegBankInfo->getValue();
 }
 
 MIRParser::MIRParser(std::unique_ptr<MIRParserImpl> Impl)
     : Impl(std::move(Impl)) {}

llvm/trunk/test/CodeGen/MIR/AArch64/register-operand-bank.mir

+# RUN: llc -o - %s -mtriple=aarch64-- -run-pass=none | FileCheck %s
+# REQUIRES: global-isel
+# Test various aspects of register bank specification on machine operands.
+--- |
+  define void @func() { ret void }
+...
+---
+# CHECK-LABEL: name: func
+# CHECK: registers:
+# CHECK:   - { id: 0, class: gpr }
+# CHECK:   - { id: 1, class: fpr }
+name: func
+body: |
+  bb.0:
+    %0 : gpr(s64) = COPY %x9
+    %x9 = COPY %0
+
+    %3 : fpr(s64) = COPY %d0
+    %d1 = COPY %3 : fpr
+...

llvm/trunk/test/CodeGen/MIR/X86/register-operand-class-invalid0.mir

+# RUN: not llc -o /dev/null %s -march=x86-64 -run-pass none 2>&1 | FileCheck %s
+# This test ensures that an error is reported for specifying the register class
+# of a physical register.
+--- |
+  define void @t() { ret void }
+...
+---
+name: t
+body: |
+  bb.0:
+    ; CHECK: [[@LINE+1]]:10: register class specification expects a virtual register
+    %eax : gr32 = COPY %rdx
+...

llvm/trunk/test/CodeGen/MIR/X86/register-operand-class-invalid1.mir

+# RUN: not llc -o /dev/null %s -march=x86-64 -run-pass none 2>&1 | FileCheck %s
+# This test ensures that an error is reported for specifying the register class
+# of a physical register.
+--- |
+  define void @t() { ret void }
+...
+---
+name: t
+body: |
+  bb.0:
+    %0 : gr32 = COPY %rdx
+    ; CHECK: [[@LINE+1]]:24: conflicting register classes, previously: GR32
+    NOOP implicit %0 : gr32_abcd
+...

llvm/trunk/test/CodeGen/MIR/X86/register-operand-class.mir

+# RUN: llc -o - %s -march=x86-64 -run-pass none | FileCheck %s
+# Test various aspects of register class specification on machine operands.
+--- |
+  define void @func() { ret void }
+...
+---
+# CHECK-LABEL: name: func
+# CHECK: registers:
+# CHECK:   - { id: 0, class: gr32 }
+# CHECK:   - { id: 1, class: gr64 }
+# CHECK:   - { id: 2, class: gr32 }
+# CHECK:   - { id: 3, class: gr16 }
+name: func
+body: |
+  bb.0:
+    %0 : gr32 = COPY %rax
+    %1.sub_32bit : gr64 = COPY %eax
+    %rdx = COPY %1
+    %2 = COPY %ecx
+    %ecx = COPY %2 : gr32
+
+    %3 : gr16 = COPY %bx
+    %bx = COPY %3 : gr16
+...