[lld][ELF] Support relocation R_AVR_LDS_STS_16 on AVRTiny devices

[benshi001]

Summary

The relocation 'R_AVR_LDS_STS_16' is introduced for the compact
16-bit 'LDS/STS' instructions on AVRTiny devices.

[benshi001]

lld/ELF/Arch/AVR.cpp

175

The assembler won't emit R_AVR_LDS_STS_16 for AVRTiny devices. And this check is hard to be tested in the unit tests.

lld/test/ELF/avr-reloc.s

93

lds/sts are 32-bit instructions on non avrtiny devices.

lld/test/ELF/avrtiny-reloc.s

2 ↗

(On Diff #481988)

The reasons why I create a new avrtiny-reloc.s, other than add my tests into the existing avr-reloc.s, are

1. Some instructions in avr-reloc.s are not supported on avrtiny devices, and the assembler will emit errors.
2. AVRTiny devices have their special relocations which will not be emitted on ordinary avr devices.

58 ↗

(On Diff #481988)

lds/sts are 16-bit instructions on avrtiny devices, while they are 32-bit on ordinary avr devices.

[benshi001]

This comment was removed by benshi001.

[benshi001]

Here is the compact LDS/STS instruction's binary format.

[benshi001]

fixes fixes https://github.com/llvm/llvm-project/issues/58876

[aykevl]

lld/test/ELF/avr-reloc.s

93

Why does the comment say R_AVR_LDS_STS_16 for atmega328p? (Sounds like a copy/paste error)

[benshi001]

lld/test/ELF/avr-reloc.s

93

Thanks. I have corrected them. llvm emits relocation R_AVR_16 for lds/sts on AVR while R_AVR_LDS_STS_16 on AVRTiny.

[benshi001]

@MaskRay Any comments on this patch ?

[MaskRay]

lld/ELF/Arch/AVR.cpp

175

calcEFlags is costly. I don't think any other arch has such an eflags based relocation check. Is it really necessary?

I suggest that you remove it, at the very least, there is no trailing dot in all lld/ELF diagnostics.

lld/test/ELF/avrtiny-reloc.s

1 ↗

(On Diff #485330)

avr-tiny*

Can avr-reloc.s be augmented to test -mcpu=attiny10?

[benshi001]

lld/ELF/Arch/AVR.cpp

175

Thanks. I have removed this unnecessary check.

lld/test/ELF/avr-reloc.s

56

Currently the disassembler can correctly decode those instructions.

lld/test/ELF/avrtiny-reloc.s

1 ↗

(On Diff #485330)

I would like to do that, but it is impossible. Since avr-reloc.s contains instructions which are invalid on AVRTiny, the assembler will report errors, so I have to create a seperate file for AVRTiny.

[benshi001]

lld/test/ELF/avrtiny-reloc.s

1 ↗

(On Diff #485330)

Actually avrtiny is not a subset of avr, though their common part is quite large, they each have own specific stuff (instruction mnemonics, instruction formats, instruction encodings, relocations, etc.).

[MaskRay]

lld/test/ELF/avrtiny-reloc.s

2 ↗

(On Diff #481988)

You can use --defsym as used by llvm/test/MC/ELF/relocation-alias.s

[benshi001]

lld/test/ELF/avrtiny-reloc.s

2 ↗

(On Diff #481988)

Thanks. I have used --defsym=tiny=1 to distinguish avr and avrtiny instructions.

[MaskRay]

lld/test/ELF/avr-reloc.s

4

The prevailing style in this directory is to place | on the previous line

53

TINY

[MaskRay]

Remove . from the subject.

[benshi001]

Thanks.

[MaskRay]

Thanks!

lld/ELF/Arch/AVR.cpp

RelExpr AVR::getRelExpr(RelType type, const Symbol &s,
   case R_AVR_MS8_LDI_NEG:
   case R_AVR_MS8_LDI:
   case R_AVR_LO8_LDI_PM:
   case R_AVR_LO8_LDI_PM_NEG:
   case R_AVR_HI8_LDI_PM:
   case R_AVR_HI8_LDI_PM_NEG:
   case R_AVR_HH8_LDI_PM:
   case R_AVR_HH8_LDI_PM_NEG:
+  case R_AVR_LDS_STS_16:
   case R_AVR_PORT5:
   case R_AVR_PORT6:
   case R_AVR_CALL:
     return R_ABS;
   case R_AVR_7_PCREL:
   case R_AVR_13_PCREL:
     return R_PC;
   default:
case R_AVR_HI8_LDI_PM_NEG:
     checkAlignment(loc, val, 2, rel);
     writeLDI(loc, (-val >> 9) & 0xff);
     break;
   case R_AVR_HH8_LDI_PM_NEG:
     checkAlignment(loc, val, 2, rel);
     writeLDI(loc, (-val >> 17) & 0xff);
     break;
 
+  case R_AVR_LDS_STS_16: {
+    checkUInt(loc, val, 7, rel);
+    const uint16_t hi = val >> 4;
+    const uint16_t lo = val & 0xf;
+    write16le(loc, (read16le(loc) & 0xf8f0) | ((hi << 8) | lo));
+    break;
+  }
+
   case R_AVR_PORT5:
     checkUInt(loc, val, 5, rel);
     write16le(loc, (read16le(loc) & 0xff07) | (val << 3));
     break;
   case R_AVR_PORT6:
     checkUInt(loc, val, 6, rel);
     write16le(loc, (read16le(loc) & 0xf9f0) | (val & 0x30) << 5 | (val & 0x0f));
     break;

lld/test/ELF/avr-reloc.s

 ; REQUIRES: avr
-; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=atmega328p %s -o %t.o
-; RUN: ld.lld %t.o --defsym=a=0x12345678 --defsym=b=30 -o %t
-; RUN: llvm-objdump -d --print-imm-hex %t | FileCheck %s
-; RUN: llvm-objdump -s %t | FileCheck --check-prefix=HEX %s
+; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=atmega328p %s -o %t0.o
+; RUN: ld.lld %t0.o --defsym=a=0x12345678 --defsym=b=30 -o %t0
+; RUN: llvm-objdump -d --print-imm-hex --mcpu=atmega328p %t0 | \
+; RUN:     FileCheck --check-prefixes=CHECK,AVR %s
+; RUN: llvm-objdump -s --mcpu=atmega328p %t0 | \
+; RUN:     FileCheck --check-prefixes=HEX,AVRHEX %s
+; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=attiny10 %s --defsym=TINY=1 -o %t1.o
+; RUN: ld.lld %t1.o --defsym=a=0x12345678 --defsym=b=30 -o %t1
+; RUN: llvm-objdump -d --print-imm-hex --mcpu=attiny10 %t1 | FileCheck %s
+; RUN: llvm-objdump -s --mcpu=attiny10 %t1 | \
+; RUN:     FileCheck --check-prefixes=HEX,TINYHEX %s
 
 .section .LDI,"ax",@progbits
 ; CHECK-LABEL: section .LDI:
 ; CHECK:       ldi     r20, 0x78
 ; CHECK-NEXT:  ldi     r20, 0x56
 ; CHECK-NEXT:  ldi     r20, 0x34
 ; CHECK-NEXT:  ldi     r20, 0x12
 ; CHECK-NEXT:  ldi     r20, 0x3c

 ldi r20, hi8(-(a))     ; R_AVR_HI8_LDI_NEG
 ldi r20, hh8(-(a))     ; R_AVR_HH8_LDI_NEG
 ldi r20, hhi8(-(a))    ; R_AVR_MS8_LDI_NEG
 
 ldi r20, pm_lo8(-(a))  ; R_AVR_LO8_LDI_PM_NEG
 ldi r20, pm_hi8(-(a))  ; R_AVR_HI8_LDI_PM_NEG
 ldi r20, pm_hh8(-(a))  ; R_AVR_HH8_LDI_PM_NEG
 
-;; The disassembler is not yet able to decode those opcodes
-;; 9e 8e    std    Y+30, r9
-;; 9e 8c    ldd    r9, Y+30
-;; 4e 96    adiw   r24, 0x1e
+.ifndef TINY
 .section .SIX,"ax",@progbits
-; HEX-LABEL: section .SIX:
-; HEX-NEXT:  9e8e9e8c 4e96
+; AVR-LABEL:    section .SIX:
+; AVR:          std   Y+30, r9
+; AVR-NEXT:     ldd   r9, Y+30
+; AVR-NEXT:     adiw  r24, 0x1e
+; AVRHEX-LABEL: section .SIX:
+; AVRHEX-NEXT:  9e8e9e8c 4e96
 std Y+b, r9   ; R_AVR_6
 ldd r9, Y+b   ; R_AVR_6
 adiw r24, b   ; R_AVR_6_ADIW
+.endif
 
 .section .PORT,"ax",@progbits
 ; CHECK-LABEL: section .PORT:
 ; CHECK:       in     r20, 0x1e
 ; CHECK-NEXT:  sbic   0x1e, 0x1
 in    r20, b  ; R_AVR_PORT6
 sbic  b, 1    ; R_AVR_PORT5
 
 ;; The disassembler is not yet able to decode those opcodes
 ;; 0f c0    rjmp   .+30
 ;; ee cf    rjmp   .-36
 ;; 69 f0    breq   .+26
 ;; 61 f3    breq   .-40
 .section .PCREL,"ax",@progbits
 ; HEX-LABEL: section .PCREL:
 ; HEX-NEXT:  0fc0eecf 69f061f3
 foo:
 rjmp foo + 32  ; R_AVR_13_PCREL
 rjmp foo - 32  ; R_AVR_13_PCREL
 breq foo + 32  ; R_AVR_7_PCREL
 breq foo - 32  ; R_AVR_7_PCREL
 
+.section .LDSSTS,"ax",@progbits
+; CHECK-LABEL: section .LDSSTS:
+; CHECK:       lds r20, 0x1e
+; CHECK-NEXT:  sts 0x1e, r21
+; HEX-LABEL:   section .LDSSTS:
+; AVRHEX:      {{.*}} 40911e00 50931e00
+; TINYHEX:     {{.*}} 4ea15ea9
+lds r20, b
+sts b, r21
+
 .section .DATA,"ax",@progbits
 ; HEX-LABEL: section .DATA:
 ; HEX-NEXT:  {{.*}} 1e1e000f 00785634 12
 .byte b        ; R_AVR_8
 .short b       ; R_AVR_16
 .short gs(b)   ; R_AVR_16_PM
 .long a        ; R_AVR_32