[M68k][AsmParser] Support parsing register masks & fix printing them

[ricky26]

Summary

Fixes PR51580.

Register masks will now be printed as 'movem.l (%sp), %a0-%a5/%d5'
for example and can now be parsed in the same format.

Previously the printed syntax was 'movem.l (%sp), %a0-%a5,%d', which
didn't match prior art and was too ambiguous to easily parse.

[myhsu]

Thanks for your swift response on adding move mask support!
overall LGTM. Please address the inline feedbacks.

llvm/lib/Target/M68k/AsmParser/M68kAsmParser.cpp

240

I think A0 ~ A7 as well as D0 ~ D7 are always contiguous (see M68kGenRegisterInfo.inc). So maybe this can be replaced with something like

if (Register >= M68k::D0 && Register <= M68k::D7)
  return Register - M68k::D0;
if (Register >= M68k::A0 && Register <= M68k::A7)
  return Register - M68k::A0 + 8;
// Here: handle SP, PC, CCR...

279

379

(formatting) remove surrounding braces for single-line if-statement

383

ditto remove braces and other places below

[myhsu]

I just found that bits in the move mask will be inverted if the destination operand is predecrement mode address (i.e. -(An)). But we're not even supporting that addressing mode for MOVEM yet so I think it's fine for now.

[ricky26]

In D108597#2961714, @myhsu wrote:

I just found that bits in the move mask will be inverted if the destination operand is predecrement mode address (i.e. -(An)). But we're not even supporting that addressing mode for MOVEM yet so I think it's fine for now.

I guess I should've called this out in the commit message, but yes. I did have a quick look and I didn't see anything for the other side of the flip (i.e. during codegen / printing) so it seems unlikely to not get caught at the point that gets implemented.

llvm/lib/Target/M68k/AsmParser/M68kAsmParser.cpp

 //===---- M68kAsmParser.cpp - Parse M68k assembly to MCInst instructions --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "M68kInstrInfo.h"
 #include "M68kRegisterInfo.h"
 #include "TargetInfo/M68kTargetInfo.h"
 
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/TargetRegistry.h"
#include "M68kGenAsmMatcher.inc"
   OperandMatchResultTy parseRegister(unsigned int &RegNo);
 
   // Parser functions.
   void eatComma();
 
   bool isExpr();
   OperandMatchResultTy parseImm(OperandVector &Operands);
   OperandMatchResultTy parseMemOp(OperandVector &Operands);
+  OperandMatchResultTy parseRegOrMoveMask(OperandVector &Operands);
 
 public:
   M68kAsmParser(const MCSubtargetInfo &STI, MCAsmParser &Parser,
                 const MCInstrInfo &MII, const MCTargetOptions &Options)
       : MCTargetAsmParser(Options, STI, MII), STI(STI), Parser(Parser) {
     MCAsmParserExtension::Initialize(Parser);
     MRI = getContext().getRegisterInfo();
 
bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
                                bool MatchingInlineAsm) override;
 };
 
 struct M68kMemOp {
   enum class Kind {
     Addr,
+    RegMask,
     Reg,
     RegIndirect,
     RegPostIncrement,
     RegPreDecrement,
     RegIndirectDisplacement,
     RegIndirectDisplacementIndex,
   };
 
   // These variables are used for the following forms:
   // Addr: (OuterDisp)
+  // RegMask: RegMask (as register mask)
   // Reg: %OuterReg
   // RegIndirect: (%OuterReg)
   // RegPostIncrement: (%OuterReg)+
   // RegPreDecrement: -(%OuterReg)
   // RegIndirectDisplacement: OuterDisp(%OuterReg)
   // RegIndirectDisplacementIndex:
   //   OuterDisp(%OuterReg, %InnerReg.Size * Scale, InnerDisp)
 
   Kind Op;
   unsigned OuterReg;
   unsigned InnerReg;
   const MCExpr *OuterDisp;
   const MCExpr *InnerDisp;
   uint8_t Size : 4;
   uint8_t Scale : 4;
   const MCExpr *Expr;
+  uint16_t RegMask;
 
   M68kMemOp() {}
   M68kMemOp(Kind Op) : Op(Op) {}
 
   void print(raw_ostream &OS) const;
 };
 
 /// An parsed M68k assembly operand.
public:
 
   // Imm
   bool isImm() const override;
   void addImmOperands(MCInst &Inst, unsigned N) const;
 
   static std::unique_ptr<M68kOperand> createImm(const MCExpr *Expr, SMLoc Start,
                                                 SMLoc End);
 
+  // MoveMask
+  bool isMoveMask() const;
+  void addMoveMaskOperands(MCInst &Inst, unsigned N) const;
+
   // Addr
   bool isAddr() const;
   bool isAddr8() const { return isAddrN<8>(); }
   bool isAddr16() const { return isAddrN<16>(); }
   bool isAddr32() const { return isAddrN<32>(); }
   void addAddrOperands(MCInst &Inst, unsigned N) const;
 
   // ARI

 
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeM68kAsmParser() {
   RegisterMCAsmParser<M68kAsmParser> X(getTheM68kTarget());
 }
 
 #define GET_MATCHER_IMPLEMENTATION
 #include "M68kGenAsmMatcher.inc"
 
+static inline unsigned getRegisterByIndex(unsigned RegisterIndex) {
+  static unsigned RegistersByIndex[] = {
+      M68k::D0, M68k::D1, M68k::D2, M68k::D3, M68k::D4, M68k::D5,
+      M68k::D6, M68k::D7, M68k::A0, M68k::A1, M68k::A2, M68k::A3,
+      M68k::A4, M68k::A5, M68k::A6, M68k::SP,
+  };
+  assert(RegisterIndex <=
+         sizeof(RegistersByIndex) / sizeof(RegistersByIndex[0]));
+  return RegistersByIndex[RegisterIndex];
+}
+
+static inline unsigned getRegisterIndex(unsigned Register) {
+  switch (Register) {
+  case M68k::D0:
+    return 0;
+  case M68k::D1:
+    return 1;
+  case M68k::D2:
+    return 2;
+  case M68k::D3:
+    return 3;
+  case M68k::D4:
+    return 4;
+  case M68k::D5:
+    return 5;
+  case M68k::D6:
+    return 6;
+  case M68k::D7:
+    return 7;
+  case M68k::A0:
+    return 8;
+  case M68k::A1:
+    return 9;
+  case M68k::A2:
+    return 10;
+  case M68k::A3:
+    return 11;
+  case M68k::A4:
+    return 12;
+  case M68k::A5:
+    return 13;
+  case M68k::A6:
+    return 14;
+  case M68k::SP:
+    return 15;
+
+  case M68k::PC:
+  case M68k::CCR:
+    return 16;
+
+  default:
+    assert(false && "unexpected register number");
+    return 0;
+  }
+}
+
 void M68kMemOp::print(raw_ostream &OS) const {
   switch (Op) {
   case Kind::Addr:
     OS << OuterDisp;
     break;
+  case Kind::RegMask:
+    OS << "RegMask(" << format("%04x", RegMask) << ")";
+    break;
   case Kind::Reg:
     OS << '%' << OuterReg;
     break;
   case Kind::RegIndirect:
     OS << "(%" << OuterReg << ')';
     break;
   case Kind::RegPostIncrement:
     OS << "(%" << OuterReg << ")+";
std::unique_ptr<M68kOperand> M68kOperand::createToken(StringRef Token,
   auto Op = std::make_unique<M68kOperand>(Kind::Token, Start, End);
   Op->Token = Token;
   return Op;
 }
 
 // Imm
 bool M68kOperand::isImm() const { return Kind == Kind::Imm; }
 void M68kOperand::addImmOperands(MCInst &Inst, unsigned N) const {
-  assert(isImm() && "wrong oeprand kind");
+  assert(isImm() && "wrong operand kind");
   assert((N == 1) && "can only handle one register operand");
 
   M68kOperand::addExpr(Inst, Expr);
 }
 
 std::unique_ptr<M68kOperand> M68kOperand::createImm(const MCExpr *Expr,
                                                     SMLoc Start, SMLoc End) {
   auto Op = std::make_unique<M68kOperand>(Kind::Imm, Start, End);
   Op->Expr = Expr;
   return Op;
 }
 
+// MoveMask
+bool M68kOperand::isMoveMask() const {
+  if (!isMemOp()) {
+    return false;
+  }
+
+  if (MemOp.Op == M68kMemOp::Kind::RegMask) {
+    return true;
+  }
+
+  if (MemOp.Op != M68kMemOp::Kind::Reg) {
+    return false;
+  }
+
+  // Only regular address / data registers are allowed to be used
+  // in register masks.
+  return getRegisterIndex(MemOp.OuterReg) < 16;
+}
+
+void M68kOperand::addMoveMaskOperands(MCInst &Inst, unsigned N) const {
+  assert(isMoveMask() && "wrong operand kind");
+  assert((N == 1) && "can only handle one immediate operand");
+
+  uint16_t MoveMask = MemOp.RegMask;
+  if (MemOp.Op == M68kMemOp::Kind::Reg) {
+    MoveMask = 1 << getRegisterIndex(MemOp.OuterReg);
+  }
+
+  Inst.addOperand(MCOperand::createImm(MoveMask));
+}
+
 // Addr
 bool M68kOperand::isAddr() const {
   return isMemOp() && MemOp.Op == M68kMemOp::Kind::Addr;
 }
 // TODO: Maybe we can also store the size of OuterDisp
 // in Size?
 template <unsigned N> bool M68kOperand::isAddrN() const {
   if (isAddr()) {
bool M68kAsmParser::parseRegisterName(unsigned &RegNo, SMLoc Loc,
   // CCR register
   if (RegisterNameLower == "ccr") {
     RegNo = M68k::CCR;
     return true;
   }
 
   // Parse simple general-purpose registers.
   if (RegisterNameLower.size() == 2) {
-    static unsigned RegistersByIndex[] = {
-        M68k::D0, M68k::D1, M68k::D2, M68k::D3, M68k::D4, M68k::D5,
-        M68k::D6, M68k::D7, M68k::A0, M68k::A1, M68k::A2, M68k::A3,
-        M68k::A4, M68k::A5, M68k::A6, M68k::SP,
-    };
 
     switch (RegisterNameLower[0]) {
     case 'd':
     case 'a': {
       if (isdigit(RegisterNameLower[1])) {
         unsigned IndexOffset = (RegisterNameLower[0] == 'a') ? 8 : 0;
         unsigned RegIndex = (unsigned)(RegisterNameLower[1] - '0');
         if (RegIndex < 8) {
-          RegNo = RegistersByIndex[IndexOffset + RegIndex];
+          RegNo = getRegisterByIndex(IndexOffset + RegIndex);
           return true;
         }
       }
       break;
     }
 
     case 's':
       if (RegisterNameLower[1] == 'p') {
OperandMatchResultTy M68kAsmParser::parseImm(OperandVector &Operands) {
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy M68kAsmParser::parseMemOp(OperandVector &Operands) {
   SMLoc Start = getLexer().getLoc();
   bool IsPD = false;
   M68kMemOp MemOp;
 
-  // Check for a plain register.
-  auto Result = parseRegister(MemOp.OuterReg);
-  if (Result == MatchOperand_Success) {
-    MemOp.Op = M68kMemOp::Kind::Reg;
-    Operands.push_back(
-        M68kOperand::createMemOp(MemOp, Start, getLexer().getLoc()));
-    return MatchOperand_Success;
-  }
-
-  if (Result == MatchOperand_ParseFail) {
+  // Check for a plain register or register mask.
+  auto Result = parseRegOrMoveMask(Operands);
+  if (Result != llvm::MatchOperand_NoMatch) {
     return Result;
   }
 
   // Check for pre-decrement & outer displacement.
   bool HasDisplacement = false;
   if (getLexer().is(AsmToken::Minus)) {
     IsPD = true;
     Parser.Lex();
OperandMatchResultTy M68kAsmParser::parseMemOp(OperandVector &Operands) {
   } else {
     MemOp.Op = M68kMemOp::Kind::RegIndirect;
   }
 
   Operands.push_back(M68kOperand::createMemOp(MemOp, Start, End));
   return MatchOperand_Success;
 }
 
+OperandMatchResultTy
+M68kAsmParser::parseRegOrMoveMask(OperandVector &Operands) {
+  SMLoc Start = getLexer().getLoc();
+  M68kMemOp MemOp(M68kMemOp::Kind::RegMask);
+  MemOp.RegMask = 0;
+
+  for (;;) {
+    bool IsFirstRegister =
+        (MemOp.Op == M68kMemOp::Kind::RegMask) && (MemOp.RegMask == 0);
+
+    unsigned FirstRegister;
+    auto Result = parseRegister(FirstRegister);
+    if (IsFirstRegister && (Result == llvm::MatchOperand_NoMatch)) {
+      return MatchOperand_NoMatch;
+    }
+    if (Result != llvm::MatchOperand_Success) {
+      Error(getLexer().getLoc(), "expected start register");
+      return MatchOperand_ParseFail;
+    }
+
+    unsigned LastRegister = FirstRegister;
+    if (getLexer().is(AsmToken::Minus)) {
+      getLexer().Lex();
+      Result = parseRegister(LastRegister);
+      if (Result != llvm::MatchOperand_Success) {
+        Error(getLexer().getLoc(), "expected end register");
+        return MatchOperand_ParseFail;
+      }
+    }
+
+    unsigned FirstRegisterIndex = getRegisterIndex(FirstRegister);
+    unsigned LastRegisterIndex = getRegisterIndex(LastRegister);
+
+    uint16_t NumNewBits = LastRegisterIndex - FirstRegisterIndex + 1;
+    uint16_t NewMaskBits = ((1 << NumNewBits) - 1) << FirstRegisterIndex;
+
+    if (IsFirstRegister && (FirstRegister == LastRegister)) {
+      // First register range is a single register, simplify to just Reg
+      // so that it matches more operands.
+      MemOp.Op = M68kMemOp::Kind::Reg;
+      MemOp.OuterReg = FirstRegister;
+    } else {
+      if (MemOp.Op == M68kMemOp::Kind::Reg) {
+        // This is the second register being specified - expand the Reg operand
+        // into a mask first.
+        MemOp.Op = M68kMemOp::Kind::RegMask;
+        MemOp.RegMask = 1 << getRegisterIndex(MemOp.OuterReg);
+
+        if (MemOp.RegMask == 0) {
+          Error(getLexer().getLoc(),
+                "special registers cannot be used in register masks");
+          return MatchOperand_ParseFail;
+        }
+      }
+
+      if ((FirstRegisterIndex >= 16) || (LastRegisterIndex >= 16)) {
+        Error(getLexer().getLoc(),
+              "special registers cannot be used in register masks");
+        return MatchOperand_ParseFail;
+      }
+
+      if (NewMaskBits & MemOp.RegMask) {
+        Error(getLexer().getLoc(), "conflicting masked registers");
+        return MatchOperand_ParseFail;
+      }
+
+      MemOp.RegMask |= NewMaskBits;
+    }
+
+    if (getLexer().isNot(AsmToken::Slash)) {
+      break;
+    }
+
+    getLexer().Lex();
+  }
+
+  Operands.push_back(
+      M68kOperand::createMemOp(MemOp, Start, getLexer().getLoc()));
+  return MatchOperand_Success;
+}
+
 void M68kAsmParser::eatComma() {
   if (Parser.getTok().is(AsmToken::Comma)) {
     Parser.Lex();
   }
 }
 
 bool M68kAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                                      SMLoc NameLoc, OperandVector &Operands) {

llvm/lib/Target/M68k/M68kInstrInfo.td

class MxBrTargetOperand<int N> : Operand<OtherVT> {
   let ParserMatchClass = !cast<AsmOperandClass>("MxAddr"#N);
 }
 // Branch targets have OtherVT type and print as pc-relative values.
 def MxBrTarget8  : MxBrTargetOperand<8>;
 def MxBrTarget16 : MxBrTargetOperand<16>;
 def MxBrTarget32 : MxBrTargetOperand<32>;
 
 // Used with MOVEM
+def MxMoveMaskClass : MxOpClass<"MoveMask">;
 def MxMoveMask : MxOp<i16, MxSize16, "m"> {
   let OperandType = "OPERAND_IMMEDIATE";
   let PrintMethod = "printMoveMask";
+  let ParserMatchClass = MxMoveMaskClass;
 }
 
 //===----------------------------------------------------------------------===//
 // Predicates
 //===----------------------------------------------------------------------===//
 
 def SmallCode    : Predicate<"TM.getCodeModel() == CodeModel::Small">;
 def KernelCode   : Predicate<"TM.getCodeModel() == CodeModel::Kernel">;

llvm/lib/Target/M68k/MCTargetDesc/M68kInstPrinter.cpp

void M68kInstPrinter::printMoveMask(const MCInst *MI, unsigned opNum,
   // separately.
   uint8_t HalfMask;
   unsigned Reg;
   for (int s = 0; s < 16; s += 8) {
     HalfMask = (Mask >> s) & 0xFF;
     // Print separation comma only if
     // both data & register parts have bit(s) set
     if (s != 0 && (Mask & 0xFF) && HalfMask)
-      O << ',';
+      O << '/';
 
     for (int i = 0; HalfMask; ++i) {
       if ((HalfMask >> i) & 0b1) {
         HalfMask ^= 0b1 << i;
         Reg = M68kII::getMaskedSpillRegister(i + s);
         printRegName(O, Reg);
 
         int j = i;
         while ((HalfMask >> (j + 1)) & 0b1)
           HalfMask ^= 0b1 << ++j;
 
         if (j != i) {
           O << '-';
           Reg = M68kII::getMaskedSpillRegister(j + s);
           printRegName(O, Reg);
         }
 
         i = j;
 
         if (HalfMask)
-          O << ',';
+          O << '/';
       }
     }
   }
 }
 
 void M68kInstPrinter::printDisp(const MCInst *MI, unsigned opNum,
                                 raw_ostream &O) {
   const MCOperand &Op = MI->getOperand(opNum);

llvm/test/CodeGen/M68k/CollapseMOVEM.mir

 # NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 # RUN: llc %s -mtriple=m68k -start-after=prologepilog -O0 -filetype=asm -o - \
 # RUN:   | FileCheck %s
 
 #------------------------------------------------------------------------------
 # CollapseMOVEM pass finds sequences of MOVEM instructions and collapse them
 # into a single instruciton with merged masks. This only works with stack data
 #------------------------------------------------------------------------------
 
 --- # CollapseMOVEM_RM
 #
 # CHECK-LABEL: CollapseMOVEM_RM
-# CHECK:       movem.l (0,%sp), %d0-%d2,%d7,%a1-%a3,%a5
+# CHECK:       movem.l (0,%sp), %d0-%d2/%d7/%a1-%a3/%a5
 name: CollapseMOVEM_RM
 body: |
   bb.0:
     MOVM32mp 1,     0, $sp
     MOVM32mp 2,     4, $sp
     MOVM32mp 4,     8, $sp
     MOVM32mp 128,  12, $sp
     MOVM32mp 512,  16, $sp
     MOVM32mp 1024, 20, $sp
     MOVM32mp 2048, 24, $sp
     MOVM32mp 8192, 28, $sp
 
 ...
 #
 # CHECK-LABEL: CollapseMOVEM_RM_Reversed
-# CHECK:       movem.l (0,%sp), %d0-%d2,%d7,%a1-%a3,%a5
+# CHECK:       movem.l (0,%sp), %d0-%d2/%d7/%a1-%a3/%a5
 name: CollapseMOVEM_RM_Reversed
 body: |
   bb.0:
     MOVM32mp 8192, 28, $sp
     MOVM32mp 2048, 24, $sp
     MOVM32mp 1024, 20, $sp
     MOVM32mp 512,  16, $sp
     MOVM32mp 128,  12, $sp
bb.0:
     MOVM32mp 4,    20, $sp
     MOVM32mp 2,    24, $sp
     MOVM32mp 1,    28, $sp
 
 ...
 --- # CollapseMOVEM_MR
 #
 # CHECK-LABEL: CollapseMOVEM_MR
-# CHECK:       movem.l %d0-%d2,%d7,%a1-%a3,%a5, (0,%sp)
+# CHECK:       movem.l %d0-%d2/%d7/%a1-%a3/%a5, (0,%sp)
 name: CollapseMOVEM_MR
 body: |
   bb.0:
     MOVM32pm   0, $sp, 1
     MOVM32pm   4, $sp, 2
     MOVM32pm   8, $sp, 4
     MOVM32pm  12, $sp, 128
     MOVM32pm  16, $sp, 512
     MOVM32pm  20, $sp, 1024
     MOVM32pm  24, $sp, 2048
     MOVM32pm  28, $sp, 8192
 
 ...
 --- # CollapseMOVEM_Mixed
 #
 # CHECK-LABEL: CollapseMOVEM_Mixed
 # CHECK:       movem.l %d0-%d1, (0,%sp)
-# CHECK:       movem.l (8,%sp), %d2,%d7
+# CHECK:       movem.l (8,%sp), %d2/%d7
 # CHECK:       movem.l %a1-%a2, (16,%sp)
 # CHECK:       movem.l (24,%sp), %a3
 # CHECK:       movem.l %a5, (28,%sp)
 name: CollapseMOVEM_Mixed
 body: |
   bb.0:
     MOVM32pm  0, $sp, 1
     MOVM32pm  4, $sp, 2

llvm/test/MC/M68k/instructions.s

 ; CHECK: rol.l #8, %d1
 rol.l #8, %d1
 ; CHECK: ror.l #8, %d1
 ror.l #8, %d1
 ; CHECK: nop
 nop
 ; CHECK: rts
 rts
+; CHECK: movem.l %d0-%d6/%a0, (%sp)
+movem.l %d0-%d6/%a0, (%sp)
+; CHECK: movem.l (10,%sp), %d0-%d6/%a0
+movem.l (10,%sp), %d0-%d6/%a0