[AVR] Fix some ambiguous cases in AsmParser

[benshi001]

Summary

Some specific operands in specific instructions should be treated
as variables/symbols/labels, other than registers.

This patch fixes those ambiguous cases, such as "lds r25, r24",
which means loading the value inside symbol 'r24' into register 'r25'.

[benshi001]

This patch fixes https://github.com/llvm/llvm-project/issues/58853 .

[benshi001]

All the tests use register names as symbols/labels/expressions, such as "jmp x + 2", "lds reg, r1", "adiw reg, y + 2".

[aykevl]

llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp

629–631

I think the following is a lot easier to read:

OperandNum++;
if (OperandNum > 1)
  eatComma();

If you want OperandNum to start at 0, I suggest initializing it to -1:

int OperandNum = -1;

650

std::array is probably a better choice in this case. See: https://reviews.llvm.org/D140569#inline-1358484

Also: can you add a test for adiw and sbiw?

651–655

Style issue: use braces for the outer if since the nested for is braced. See: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

for (auto Inst : Insts) {
  if (Inst == Mnemonic) {
    maybeReg = false;
    break;
  }
}

Same for the loop below.

[benshi001]

llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp

650

Done. Changed to std::array, and also tests for adiw/sbiw are added.

[benshi001]

llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp

629–631

Thanks. Using int OperandNum = -1; looks more clear.

[benshi001]

llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp

658

there may be other instructions which need a careful check. So I temporarily give the array a length=8.

llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp

 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/MathExtras.h"
 
+#include <array>
 #include <sstream>
 
 #define DEBUG_TYPE "avr-asm-parser"
 
 using namespace llvm;
 
 namespace {
 /// Parses AVR assembly from a stream.
#include "AVRGenAsmMatcher.inc"
 
   bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override;
 
   bool ParseDirective(AsmToken DirectiveID) override;
 
   OperandMatchResultTy parseMemriOperand(OperandVector &Operands);
 
-  bool parseOperand(OperandVector &Operands);
+  bool parseOperand(OperandVector &Operands, bool maybeReg);
   int parseRegisterName(unsigned (*matchFn)(StringRef));
   int parseRegisterName();
   int parseRegister(bool RestoreOnFailure = false);
   bool tryParseRegisterOperand(OperandVector &Operands);
   bool tryParseExpression(OperandVector &Operands);
   bool tryParseRelocExpression(OperandVector &Operands);
   void eatComma();
 
bool AVRAsmParser::tryParseRelocExpression(OperandVector &Operands) {
   // in accordance with avr-gcc.
   AsmToken::TokenKind CurTok = Parser.getLexer().getKind();
   if (CurTok == AsmToken::Minus || CurTok == AsmToken::Plus)
     return true;
 
   // Check for sign.
   AsmToken tokens[2];
   if (Parser.getLexer().peekTokens(tokens) == 2)
-      if (tokens[0].getKind() == AsmToken::LParen &&
-          tokens[1].getKind() == AsmToken::Minus)
-        isNegated = true;
 
   // Check if we have a target specific modifier (lo8, hi8, &c)
   if (CurTok != AsmToken::Identifier ||
       Parser.getLexer().peekTok().getKind() != AsmToken::LParen) {
     // Not a reloc expr
     return true;
   }
   StringRef ModifierName = Parser.getTok().getString();
MCExpr const *Expression =
       AVRMCExpr::create(ModifierKind, InnerExpression, isNegated, getContext());
 
   SMLoc E = SMLoc::getFromPointer(Parser.getTok().getLoc().getPointer() - 1);
   Operands.push_back(AVROperand::CreateImm(Expression, S, E));
 
   return false;
 }
 
-bool AVRAsmParser::parseOperand(OperandVector &Operands) {
+bool AVRAsmParser::parseOperand(OperandVector &Operands, bool maybeReg) {
   LLVM_DEBUG(dbgs() << "parseOperand\n");
 
   switch (getLexer().getKind()) {
   default:
     return Error(Parser.getTok().getLoc(), "unexpected token in operand");
 
   case AsmToken::Identifier:
-    // Try to parse a register, if it fails,
-    // fall through to the next case.
-    if (!tryParseRegisterOperand(Operands)) {
+    // Try to parse a register, fall through to the next case if it fails.
+    if (maybeReg && !tryParseRegisterOperand(Operands)) {
       return false;
     }
     [[fallthrough]];
   case AsmToken::LParen:
   case AsmToken::Integer:
   case AsmToken::Dot:
     return tryParseExpression(Operands);
   case AsmToken::Plus:
void AVRAsmParser::eatComma() {
   }
 }
 
 bool AVRAsmParser::ParseInstruction(ParseInstructionInfo &Info,
                                     StringRef Mnemonic, SMLoc NameLoc,
                                     OperandVector &Operands) {
   Operands.push_back(AVROperand::CreateToken(Mnemonic, NameLoc));
 
-  bool first = true;
+  int OperandNum = -1;
   while (getLexer().isNot(AsmToken::EndOfStatement)) {
-    if (!first)
+    OperandNum++;
+    if (OperandNum > 0)
       eatComma();
 
-    first = false;
-
     auto MatchResult = MatchOperandParserImpl(Operands, Mnemonic);
 
     if (MatchResult == MatchOperand_Success) {
       continue;
     }
 
     if (MatchResult == MatchOperand_ParseFail) {
       SMLoc Loc = getLexer().getLoc();
       Parser.eatToEndOfStatement();
 
       return Error(Loc, "failed to parse register and immediate pair");
     }
 
-    if (parseOperand(Operands)) {
+    // These specific operands should be treated as addresses/symbols/labels,
+    // other than registers.
+    bool maybeReg = true;
+    if (OperandNum == 1) {
+      std::array<StringRef, 8> Insts = {"lds", "adiw", "sbiw", "ldi"};
+      for (auto Inst : Insts) {
+        if (Inst == Mnemonic) {
+          maybeReg = false;
+          break;
+        }
+      }
+    } else if (OperandNum == 0) {
+      std::array<StringRef, 8> Insts = {"sts", "call", "rcall", "rjmp", "jmp"};
+      for (auto Inst : Insts) {
+        if (Inst == Mnemonic) {
+          maybeReg = false;
+          break;
+        }
+      }
+    }
+
+    if (parseOperand(Operands, maybeReg)) {
       SMLoc Loc = getLexer().getLoc();
       Parser.eatToEndOfStatement();
       return Error(Loc, "unexpected token in argument list");
     }
   }
   Parser.Lex(); // Consume the EndOfStatement
   return false;
 }

llvm/test/MC/AVR/inst-adiw.s

foo:
 
   adiw r30,  3
   adiw r30,  63
   adiw r24,  63
   adiw r24,  0
   adiw r30,  0
 
   adiw r24, SYMBOL
+  adiw r24, r25
 
 ; CHECK: adiw r26,  12               ; encoding: [0x1c,0x96]
 ; CHECK: adiw r26,  63               ; encoding: [0xdf,0x96]
 
 ; CHECK: adiw r28,  17               ; encoding: [0x61,0x96]
 ; CHECK: adiw r28,  0                ; encoding: [0x20,0x96]
 
 ; CHECK: adiw r30,  3                ; encoding: [0x33,0x96]
 ; CHECK: adiw r30,  63               ; encoding: [0xff,0x96]
 ; CHECK: adiw r24,  63               ; encoding: [0xcf,0x96]
 ; CHECK: adiw r24,  0                ; encoding: [0x00,0x96]
 ; CHECK: adiw r30,  0                ; encoding: [0x30,0x96]
 
 ; CHECK: adiw r24, SYMBOL            ; encoding: [0b00AAAAAA,0x96]
                                      ;   fixup A - offset: 0, value: SYMBOL, kind: fixup_6_adiw
+; CHECK: adiw r24, r25               ; encoding: [0b00AAAAAA,0x96]
+                                     ;   fixup A - offset: 0, value: r25, kind: fixup_6_adiw
 
 ; CHECK-INST: adiw r26,  12
 ; CHECK-INST: adiw r26,  63
 
 ; CHECK-INST: adiw r28,  17
 ; CHECK-INST: adiw r28,  0
 
 ; CHECK-INST: adiw r30,  3
 ; CHECK-INST: adiw r30,  63
 ; CHECK-INST: adiw r24,  63
 ; CHECK-INST: adiw r24,  0
 ; CHECK-INST: adiw r30,  0
 
 ; CHECK-INST: adiw r24, 0
 ; CHECK-INST:      R_AVR_6_ADIW SYMBOL
+; CHECK-INST: adiw r24, 0
+; CHECK-INST:      R_AVR_6_ADIW r25

llvm/test/MC/AVR/inst-call.s

 ; RUN: llvm-mc -triple avr -mattr=jmpcall -show-encoding < %s | FileCheck %s
 ; RUN: llvm-mc -filetype=obj -triple avr -mattr=jmpcall < %s | llvm-objdump --no-print-imm-hex -dr --mattr=jmpcall - | FileCheck -check-prefix=CHECK-INST %s
 
 
 foo:
 
   call  4096
   call  -124
   call   -12
   call   0
 
+r25:
+  call  r25
+
 ; CHECK: call  4096                 ; encoding: [0x0e,0x94,0x00,0x08]
 ; CHECK: call -124                  ; encoding: [0xff,0x95,0xc2,0xff]
 ; CHECK: call -12                   ; encoding: [0xff,0x95,0xfa,0xff]
 ; CHECK: call  0                    ; encoding: [0x0e,0x94,0x00,0x00]
-
+; CHECK: call  r25                  ; encoding: [0x0e'A',0x94'A',0b00AAAAAA,0x00]
+; CHECK:                            ;   fixup A - offset: 0, value: r25, kind: fixup_call
 
 ; CHECK-INST: call 4096
 ; CHECK-INST: call 8388484
 ; CHECK-INST: call 8388596
 ; CHECK-INST: call 0
+; CHECK-INST: call 0
+; CHECK-INST:      R_AVR_CALL

llvm/test/MC/AVR/inst-lds-tiny.s

 ; RUN: llvm-mc -triple avr -mattr=sram,tinyencoding -show-encoding < %s | FileCheck %s
 ; RUN: llvm-mc -filetype=obj -triple avr -mattr=sram,tinyencoding < %s | llvm-objdump --no-print-imm-hex -dr --mattr=sram,tinyencoding - | FileCheck -check-prefix=CHECK-INST %s
 
 foo:
   lds r16, 113
   lds r29, 62
   lds r22, 44
   lds r27, 92
   lds r20, SYMBOL+12
+  lds r20, r21
+  lds r20, z+6
 
 ; CHECK:  lds r16, 113                 ; encoding: [0x01,0xa7]
 ; CHECK:  lds r29, 62                  ; encoding: [0xde,0xa3]
 ; CHECK:  lds r22, 44                  ; encoding: [0x6c,0xa2]
 ; CHECK:  lds r27, 92                  ; encoding: [0xbc,0xa5]
 ; CHECK:  lds r20, SYMBOL+12           ; encoding: [0x40'A',0xa0'A']
 ; CHECK:                               ; fixup A - offset: 0, value: SYMBOL+12, kind: fixup_lds_sts_16
+; CHECK:  lds r20, r21                 ; encoding: [0x40'A',0xa0'A']
+; CHECK:                               ; fixup A - offset: 0, value: r21, kind: fixup_lds_sts_16
+; CHECK:  lds r20, z+6                 ; encoding: [0x40'A',0xa0'A']
+; CHECK:                               ; fixup A - offset: 0, value: z+6, kind: fixup_lds_sts_16
 
 ; CHECK-INST: lds r16, 113
 ; CHECK-INST: lds r29, 62
 ; CHECK-INST: lds r22, 44
 ; CHECK-INST: lds r27, 92
-; CHECK-INST: lds r20,  0
-; CHECK-INST: R_AVR_LDS_STS_16 SYMBOL+0xc
+; CHECK-INST: lds r20, 0
+; CHECK-INST:          R_AVR_LDS_STS_16 SYMBOL+0xc
+; CHECK-INST: lds r20, 0
+; CHECK-INST:          R_AVR_LDS_STS_16 r21
+; CHECK-INST: lds r20, 0
+; CHECK-INST:          R_AVR_LDS_STS_16 z+0x6

llvm/test/MC/AVR/inst-lds.s

 ; RUN: llvm-mc -triple avr -mattr=sram -show-encoding < %s | FileCheck %s
 ; RUN: llvm-mc -filetype=obj -triple avr -mattr=sram < %s | llvm-objdump --no-print-imm-hex -dr --mattr=sram - | FileCheck -check-prefix=CHECK-INST %s
 
 
 foo:
   lds r16, 241
   lds r29, 190
   lds r22, 172
   lds r27, 92
-  lds r4, SYMBOL+12
+  lds r4,  SYMBOL+12
+  lds r4,  r25
+  lds r4,  x+2
 
 ; CHECK: lds r16, 241                 ; encoding: [0x00,0x91,0xf1,0x00]
 ; CHECK: lds r29, 190                 ; encoding: [0xd0,0x91,0xbe,0x00]
 ; CHECK: lds r22, 172                 ; encoding: [0x60,0x91,0xac,0x00]
 ; CHECK: lds r27, 92                  ; encoding: [0xb0,0x91,0x5c,0x00]
-; CHECK: lds r4, SYMBOL+12            ; encoding: [0x40,0x90,A,A]
+; CHECK: lds r4,  SYMBOL+12           ; encoding: [0x40,0x90,A,A]
 ; CHECK:                              ;   fixup A - offset: 2, value: SYMBOL+12, kind: fixup_16
-
+; CHECK: lds r4,  r25                 ; encoding: [0x40,0x90,A,A]
+; CHECK:                              ;   fixup A - offset: 2, value: r25, kind: fixup_16
+; CHECK: lds r4,  x+2                 ; encoding: [0x40,0x90,A,A]
+; CHECK:                              ;   fixup A - offset: 2, value: x+2, kind: fixup_16
 
 ; CHECK-INST: lds r16, 241
 ; CHECK-INST: lds r29, 190
 ; CHECK-INST: lds r22, 172
 ; CHECK-INST: lds r27, 92
 ; CHECK-INST: lds r4,  0
-; CHECK-INST:     R_AVR_16 SYMBOL+0xc
+; CHECK-INST:          R_AVR_16 SYMBOL+0xc
+; CHECK-INST: lds r4,  0
+; CHECK-INST:          R_AVR_16 r25
+; CHECK-INST: lds r4,  0
+; CHECK-INST:          R_AVR_16 x+0x2

llvm/test/MC/AVR/inst-rjmp.s

 ; RUN: llvm-mc -triple avr -show-encoding < %s | FileCheck %s
 ; RUN: llvm-mc -filetype=obj -triple avr < %s \
 ; RUN:     | llvm-objdump -d - | FileCheck --check-prefix=INST %s
 
 
 foo:
 
   rjmp  .+2
   rjmp  .-2
   rjmp  foo
   rjmp  .+8
   rjmp  end
   rjmp  .+0
 end:
   rjmp .-4
   rjmp .-6
+x:
+  rjmp x
 
 ; CHECK: rjmp    .Ltmp0+2                ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: .Ltmp0+2, kind: fixup_13_pcrel
 ; CHECK: rjmp    .Ltmp1-2                ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: .Ltmp1-2, kind: fixup_13_pcrel
 ; CHECK: rjmp    foo                     ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: foo, kind: fixup_13_pcrel
 ; CHECK: rjmp    .Ltmp2+8                ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: .Ltmp2+8, kind: fixup_13_pcrel
 ; CHECK: rjmp    end                     ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: end, kind: fixup_13_pcrel
 ; CHECK: rjmp    .Ltmp3+0                ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: .Ltmp3+0, kind: fixup_13_pcrel
 ; CHECK: rjmp    .Ltmp4-4                ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: .Ltmp4-4, kind: fixup_13_pcrel
 ; CHECK: rjmp    .Ltmp5-6                ; encoding: [A,0b1100AAAA]
 ; CHECK:                                 ;   fixup A - offset: 0, value: .Ltmp5-6, kind: fixup_13_pcrel
+; CHECK: rjmp    x                       ; encoding: [A,0b1100AAAA]
+; CHECK:                                 ;   fixup A - offset: 0, value: x, kind: fixup_13_pcrel
 
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
 ; INST: rjmp  .+0
+; INST: rjmp  .+0

llvm/test/MC/AVR/inst-sbiw.s

foo:
 
   sbiw r30, 63
   sbiw Z,   47
 
   sbiw r24, 1
   sbiw r24, 2
 
   sbiw r24, SYMBOL-1
+  sbiw r24, z+15
 
 ; CHECK: sbiw r26,  54                 ; encoding: [0xd6,0x97]
 ; CHECK: sbiw r26,  63                 ; encoding: [0xdf,0x97]
 
 ; CHECK: sbiw r28,  52                 ; encoding: [0xe4,0x97]
 ; CHECK: sbiw r28,  0                  ; encoding: [0x20,0x97]
 
 ; CHECK: sbiw r30,  63                 ; encoding: [0xff,0x97]
 ; CHECK: sbiw r30,  47                 ; encoding: [0xbf,0x97]
 
 ; CHECK: sbiw r24,  1                  ; encoding: [0x01,0x97]
 ; CHECK: sbiw r24,  2                  ; encoding: [0x02,0x97]
 
 ; CHECK: sbiw    r24, SYMBOL-1         ; encoding: [0b00AAAAAA,0x97]
                                        ;   fixup A - offset: 0, value: SYMBOL-1, kind: fixup_6_adiw
+; CHECK: sbiw    r24, z+15             ; encoding: [0b00AAAAAA,0x97]
+                                       ;   fixup A - offset: 0, value: z+15, kind: fixup_6_adiw
 
 ; CHECK-INST: sbiw r26, 54
 ; CHECK-INST: sbiw r26, 63
 
 ; CHECK-INST: sbiw r28, 52
 ; CHECK-INST: sbiw r28, 0
 
 ; CHECK-INST: sbiw r30, 63
 ; CHECK-INST: sbiw r30, 47
 
 ; CHECK-INST: sbiw r24, 1
 ; CHECK-INST: sbiw r24, 2
 
 ; CHECK-INST: sbiw r24, 0
 ; CHECK-INST:      R_AVR_6_ADIW SYMBOL-0x1
+; CHECK-INST: sbiw r24, 0
+; CHECK-INST:      R_AVR_6_ADIW z+0xf

llvm/test/MC/AVR/inst-sts-tiny.s

 ; RUN: llvm-mc -triple avr -mattr=sram,tinyencoding -show-encoding < %s | FileCheck %s
 ; RUN: llvm-mc -filetype=obj -triple avr -mattr=sram,tinyencoding < %s | llvm-objdump --no-print-imm-hex -dr --mattr=sram,tinyencoding - | FileCheck -check-prefix=CHECK-INST %s
 
 foo:
   sts 3,        r16
   sts 127,      r17
   sts SYMBOL+1, r25
+  sts x,        r25
+  sts r25+1,    r25
 
 ; CHECK:  sts 3,        r16           ; encoding: [0x03,0xa8]
 ; CHECK:  sts 127,      r17           ; encoding: [0x1f,0xaf]
 ; CHECK:  sts SYMBOL+1, r25           ; encoding: [0x90'A',0xa8'A']
 ; CHECK:                              ; fixup A - offset: 0, value: SYMBOL+1, kind: fixup_lds_sts_16
+; CHECK:  sts x,        r25           ; encoding: [0x90'A',0xa8'A']
+; CHECK:                              ; fixup A - offset: 0, value: x, kind: fixup_lds_sts_16
+; CHECK:  sts r25+1,    r25           ; encoding: [0x90'A',0xa8'A']
+; CHECK:                              ; fixup A - offset: 0, value: r25+1, kind: fixup_lds_sts_16
 
 ; CHECK-INST: sts 3,   r16
 ; CHECK-INST: sts 127, r17
 ; CHECK-INST: sts 0,   r25
-; CHECK-INST: R_AVR_LDS_STS_16 SYMBOL+0x1
+; CHECK-INST:     R_AVR_LDS_STS_16 SYMBOL+0x1
+; CHECK-INST: sts 0,   r25
+; CHECK-INST:     R_AVR_LDS_STS_16 x
+; CHECK-INST: sts 0,   r25
+; CHECK-INST:     R_AVR_LDS_STS_16 r25+0x1

llvm/test/MC/AVR/inst-sts.s

 ; RUN: llvm-mc -triple avr -mattr=sram -show-encoding < %s | FileCheck %s
 ; RUN: llvm-mc -filetype=obj -triple avr -mattr=sram < %s | llvm-objdump --no-print-imm-hex -dr --mattr=sram - | FileCheck -check-prefix=CHECK-INST %s
 
 
 foo:
 
-  sts 3,   r5
-  sts 255, r7
+  sts 3,        r5
+  sts 255,      r7
   sts SYMBOL+1, r25
+  sts r25,      r25
+  sts y+3,      r25
 
-; CHECK:  sts 3,   r5                 ; encoding: [0x50,0x92,0x03,0x00]
-; CHECK:  sts 255, r7                 ; encoding: [0x70,0x92,0xff,0x00]
+; CHECK:  sts 3,        r5            ; encoding: [0x50,0x92,0x03,0x00]
+; CHECK:  sts 255,      r7            ; encoding: [0x70,0x92,0xff,0x00]
 ; CHECK:  sts SYMBOL+1, r25           ; encoding: [0x90,0x93,A,A]
 ; CHECK:                              ;   fixup A - offset: 2, value: SYMBOL+1, kind: fixup_16
+; CHECK:  sts r25,      r25           ; encoding: [0x90,0x93,A,A]
+; CHECK:                              ;   fixup A - offset: 2, value: r25, kind: fixup_16
+; CHECK:  sts y+3,      r25           ; encoding: [0x90,0x93,A,A]
+; CHECK:                              ;   fixup A - offset: 2, value: y+3, kind: fixup_16
 
 
-; CHECK-INST: sts 3, r5
+; CHECK-INST: sts 3,   r5
 ; CHECK-INST: sts 255, r7
-; CHECK-INST: sts 0, r25
+; CHECK-INST: sts 0,   r25
 ; CHECK-INST:     R_AVR_16 SYMBOL+0x1
+; CHECK-INST: sts 0,   r25
+; CHECK-INST:     R_AVR_16 r25
+; CHECK-INST: sts 0,   r25
+; CHECK-INST:     R_AVR_16 y+0x3