[ELF] Implement the relocations of AVR for LLD

[xiangzhai]

Summary

Hi LLVM developers,

According to http://www.atmel.com/images/Atmel-0856-AVR-Instruction-Set-Manual.pdf

1	1	1	0	K K K K	d d d d	K K K K	LDI Rd,K

basic-avr.o was generated by avr-gcc:

$ avr-gcc -mmcu=atmega328p -o basic-avr.o -c basic-avr.s

then link with avr-ld

$ avr-objdump -d basic-avr-avr-ld

basic-avr-avr-ld:     file format elf32-avr


Disassembly of section .text:

00000000 <__ctors_end>:
   0:   0e 94 05 00     call    0xa     ; 0xa <foo>
   4:   1a e0           ldi     r17, 0x0A       ; 10
   6:   20 e0           ldi     r18, 0x00       ; 0
   8:   3a e0           ldi     r19, 0x0A       ; 10

0000000a <foo>:
   a:   0c 94 05 00     jmp     0xa     ; 0xa <foo>

0000000e <bar>:
   e:   0e 94 05 00     call    0xa     ; 0xa <foo>

is same with LLD:

$ avr-objdump -d basic-avr-lld 

basic-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000000 <main>:
   0:   0e 94 05 00     call    0xa     ; 0xa <foo>
   4:   1a e0           ldi     r17, 0x0A       ; 10
   6:   20 e0           ldi     r18, 0x00       ; 0
   8:   3a e0           ldi     r19, 0x0A       ; 10

0000000a <foo>:
   a:   0c 94 05 00     jmp     0xa     ; 0xa <foo>

0000000e <bar>:
   e:   0e 94 05 00     call    0xa     ; 0xa <foo>

please review my patch, thanks a lot!

PS: do I need to override getImplicitAddend for R_PC type relocation, for example: R_AVR_7_PCREL?

Regards,
Leslie Zhai

[ruiu]

ELF/Arch/AVR.cpp

101

read<uint8_t> or write<uint8_t> don't make sense because byte order doesn't matter if you read or write a single byte.

193

Why did you change this?

[ruiu]

Even though I do not fully understand what linkers should do for AVR, I don't think this patch is correct and ready for code review. For example, doing nothing for R_AVR_HI8_LDI seems apparently odd. I believe it happens to generate the same result as ld.bfd because in the given test case, high address happens to be zero.

Firstly, can you give me a documentation about AVR relocations? I cannot review this patch without knowing the relocations this patch is trying to implement.

Secondly, can you review your patch yourself until you think everything perfectly makes sense? Obviously, not implementing R_AVR_HI8_LDI is odd, for example, and I don't have enough time to investigate all questions that raised when I was reading your patch (I did that this time to see if R_AVR_HI8_LDI is not a nop relocation though). Last AVR patch (https://reviews.llvm.org/D32991) took 70+ messages and more than a month to review despite its smallness, and I don't want both of us to spend that much time this time.

[xiangzhai]

Dear Rui,

幸せな先生の日！

doing nothing for R_AVR_HI8_LDI seems apparently odd.

00000000 <main>:
   0:   0e 94 00 00     call    0       ; 0x0 <main>
   4:   10 e0           ldi     r17, 0x00       ; 0
   6:   20 e0           ldi     r18, 0x00       ; 0
   8:   30 e0           ldi     r19, 0x00       ; 0

0000000a <foo>:
   a:   0c 94 00 00     jmp     0       ; 0x0 <main>

relocated to by avr-ld:

00000000 <__ctors_end>:
   0:   0e 94 05 00     call    0xa     ; 0xa <foo>
   4:   1a e0           ldi     r17, 0x0A       ; 10
   6:   20 e0           ldi     r18, 0x00       ; 0 <-- *without* change at all?
   8:   3a e0           ldi     r19, 0x0A       ; 10

0000000a <foo>:
   a:   0c 94 05 00     jmp     0xa     ; 0xa <foo>

so I argue that it doesn't need to be relocated (doing nothing) for R_AVR_HI8_LDI, sorry for my foolish...

Firstly, can you give me a documentation about AVR relocations?

There is no exp about AVR relocation in the http://www.atmel.com/images/Atmel-0856-AVR-Instruction-Set-Manual.pdf no even such lesson in my assembly language or compiler principle courses 15 years ago, but only binutils's source code about it.

D32991 I can *not* understand your relocation method about 0e 94 00 00 -> 0e 94 05 00 for 0xa relocation address. why not write16le(Loc, read16le(Loc)); for 0e 94?

took 70+ messages and more than a month to review despite its smallness, and I don't want both of us to spend that much time this time.

先生 sorry for wasting your time! I need to go deepin to binutils to learn how to relocate, thanks again for your help Orz

Regards,
Leslie Zhai

[ruiu]

doing nothing for R_AVR_HI8_LDI seems apparently odd.

00000000 <main>:
   0:   0e 94 00 00     call    0       ; 0x0 <main>
   4:   10 e0           ldi     r17, 0x00       ; 0
   6:   20 e0           ldi     r18, 0x00       ; 0
   8:   30 e0           ldi     r19, 0x00       ; 0

0000000a <foo>:
   a:   0c 94 00 00     jmp     0       ; 0x0 <main>

relocated to by avr-ld:

00000000 <__ctors_end>:
   0:   0e 94 05 00     call    0xa     ; 0xa <foo>
   4:   1a e0           ldi     r17, 0x0A       ; 10
   6:   20 e0           ldi     r18, 0x00       ; 0 <-- *without* change at all?
   8:   3a e0           ldi     r19, 0x0A       ; 10

0000000a <foo>:
   a:   0c 94 05 00     jmp     0xa     ; 0xa <foo>

so I argue that it doesn't need to be relocated (doing nothing) for R_AVR_HI8_LDI, sorry for my foolish...

As I wrote, "I believe it happens to generate the same result as ld.bfd because in the given test case, high address happens to be zero," and I'm still believing that that is the case. A relocation that doesn't really do anything is very odd, and it is likely a sign that something's not correct. I believe you want to learn/investigate more about ELF/AVR to write code for an ELF/AVR linker. You need to be able to explain with certainty what that relocation is and what it should do.

[xiangzhai]

Dear Rui,

I believe you want to learn/investigate more about ELF/AVR to write code for an ELF/AVR linker.

Yes 先生 :) and I read ld.bfd's elf32-avr.c avr_final_link_relocate, then re-implement the relocation of LDI and other Instructions for LLD, tested by make check-lld and compared with avr-ld, the result is same:

$ avr-gcc -mmcu=atmega328p -o basic-avr.o -c basic-avr.s
$ avr-readelf -r basic-avr.o 

Relocation section '.rela.text' at offset 0xe4 contains 10 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000106 R_AVR_LO8_LDI     00000000   .text + 12
00000002  00000107 R_AVR_HI8_LDI     00000000   .text + 16
00000004  00000108 R_AVR_HH8_LDI     00000000   .text + 12
00000006  00000113 R_AVR_LDI         00000000   .text + 12
00000008  00000102 R_AVR_7_PCREL     00000000   .text + e
0000000a  00000103 R_AVR_13_PCREL    00000000   .text + e
0000000c  00000115 R_AVR_6_ADIW      00000000   .text + 16
0000000e  00000112 R_AVR_CALL        00000000   .text + e
00000012  00000112 R_AVR_CALL        00000000   .text + 12
00000016  00000112 R_AVR_CALL        00000000   .text + 16

relocated by avr-ld:

$ avr-ld -o basic-avr basic-avr.o -Ttext=0
$ avr-objdump -d basic-avr

basic-avr:     file format elf32-avr


Disassembly of section .text:

00000000 <__ctors_end>:
   0:   12 e1           ldi     r17, 0x12       ; 18
   2:   20 e0           ldi     r18, 0x00       ; 0
   4:   30 e0           ldi     r19, 0x00       ; 0
   6:   42 e1           ldi     r20, 0x12       ; 18
   8:   11 f4           brne    .+4             ; 0xe <next>
   a:   01 c0           rjmp    .+2             ; 0xe <next>
   c:   46 96           adiw    r24, 0x16       ; 22

0000000e <next>:
   e:   0c 94 07 00     jmp     0xe     ; 0xe <next>

00000012 <foo>:
  12:   0c 94 09 00     jmp     0x12    ; 0x12 <foo>

00000016 <bar>:
  16:   0c 94 0b 00     jmp     0x16    ; 0x16 <bar>

compared with LLD, the result is same:

$ llvm/build/bin/ld.lld -o basic-avr-lld basic-avr.o -Ttext=0
$ avr-objdump -d basic-avr-lld

basic-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000000 <main>:
   0:   12 e1           ldi     r17, 0x12       ; 18
   2:   20 e0           ldi     r18, 0x00       ; 0
   4:   30 e0           ldi     r19, 0x00       ; 0
   6:   42 e1           ldi     r20, 0x12       ; 18
   8:   11 f4           brne    .+4             ; 0xe <next>
   a:   01 c0           rjmp    .+2             ; 0xe <next>
   c:   46 96           adiw    r24, 0x16       ; 22

0000000e <next>:
   e:   0c 94 07 00     jmp     0xe     ; 0xe <next>

00000012 <foo>:
  12:   0c 94 09 00     jmp     0x12    ; 0x12 <foo>

00000016 <bar>:
  16:   0c 94 0b 00     jmp     0x16    ; 0x16 <bar>

先生 please review my patch, thanks a lot!

Regards,
Leslie Zhai

[ruiu]

This patch still doesn't look like it is ready for code review.

Can you add more code to the test file so that all relocations are covered by the test? The number of relocations generated by the test is apparently less than the number of the relocations you are implementing in this patch.

[dylanmckay]

ELF/Arch/AVR.cpp

45

s/caculate/calculate

98

I think we should make this into a method - all PC-relative instructions need to be adjusted by -2 IIRC to account for the size of the current instruction.

We can use this function in the other cases in this method, and also add a comment to the new method itself describing that all instructions that are pc-relative are 16-bit instructions, which is why it is 2 and not 4.

Consider the equivalent code in LLVM itself here, which is very readable

104

Can we move the rightshifting functionality out into a separate method with a comment?

This is required because all instructions are 2 or 4 bytes, and so every single instruction resides at an even byte offset (implies bit 0 is always 0). Because of this, AVR addresses in these instructions are rightshifted by one to gain an extra addressing bit.

110

I believe we need to mask out all the upper bits for LO8.

150

Double semicolon

176

I think there's a difference between these two relocations

[xiangzhai]

Dear Dylan,

How is going? long time no see :)

I updated my patch as your suggestion, and I follow your testcase to cover all relocations, but I have no idea about:
1. R_AVR_LO8_LDI_GS
it equals to R_AVR_LO8_LDI_PM, so do I need to test it lo8_ldi_gs?
2. R_AVR_16_PM
3. R_AVR_LDS_STS_16
sts foo, r25 failed to generate such reloc?

R_AVR_LDS_STS_16 is 7 bit immediate for LDS/STS in *Tiny core*:

$ cat tiny-avr.s 
main:
  lds r17, foo
foo:
  rjmp foo

$ avr-gcc -mmcu=attiny4 -o tiny-avr.o -c tiny-avr.s
$ avr-readelf -r tiny-avr.o

Relocation section '.rela.text' at offset 0xa4 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000121 R_AVR_LDS_STS_16  00000000   .text + 2
00000002  00000103 R_AVR_13_PCREL    00000000   .text + 2

workaround for avr-ld:

$ avr-ld --no-warn-mismatch -o tiny-avr tiny-avr.o -Ttext=40
$ avr-objdump -d tiny-avr

tiny-avr:     file format elf32-avr


Disassembly of section .text:

00000040 <__ctors_end>:
  40:   12 a1           ldd     r17, Z+34       ; 0x22

00000042 <foo>:
  42:   ff cf           rjmp    .-2             ; 0x42 <foo>

LLD is able to work :)

$ llvm/build/bin/ld.lld -o tiny-avr-lld tiny-avr.o -Ttext=40
$ avr-objdump -d tiny-avr-lld 

tiny-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000040 <main>:
  40:   12 a1           ldd     r17, Z+34       ; 0x22

00000042 <foo>:
  42:   ff cf           rjmp    .-2             ; 0x42 <foo>

Dear Rui,

Can you add more code to the test file so that all relocations are covered by the test? The number of relocations generated by the test is apparently less than the number of the relocations you are implementing in this patch.

I added more testcase to cover others:

$ avr-readelf -r basic-avr.o

Relocation section '.rela.text' at offset 0xdc contains 23 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000112 R_AVR_CALL        00000000   .text + 26
00000004  00000106 R_AVR_LO8_LDI     00000000   .text + 26
00000006  00000107 R_AVR_HI8_LDI     00000000   .text + 26
00000008  00000108 R_AVR_HH8_LDI     00000000   .text + 26
0000000a  00000116 R_AVR_MS8_LDI     00000000   .text + 26
0000000c  00000113 R_AVR_LDI         00000000   .text + 26
0000000e  00000109 R_AVR_LO8_LDI_NEG 00000000   .text + 26
00000010  0000010a R_AVR_HI8_LDI_NEG 00000000   .text + 26
00000012  0000010b R_AVR_HH8_LDI_NEG 00000000   .text + 26
00000014  00000117 R_AVR_MS8_LDI_NEG 00000000   .text + 26
00000016  0000010c R_AVR_LO8_LDI_PM  00000000   .text + 26
00000018  0000010d R_AVR_HI8_LDI_PM  00000000   .text + 26
0000001a  0000010e R_AVR_HH8_LDI_PM  00000000   .text + 26
0000001c  0000010f R_AVR_LO8_LDI_PM_NEG 00000000   .text + 26
0000001e  00000110 R_AVR_HI8_LDI_PM_NEG 00000000   .text + 26
00000020  00000111 R_AVR_HH8_LDI_PM_NEG 00000000   .text + 26
00000022  00000102 R_AVR_7_PCREL     00000000   .text + 26
00000024  00000103 R_AVR_13_PCREL    00000000   .text + 26
0000002e  00000114 R_AVR_6           00000000   .text + 26
00000032  00000104 R_AVR_16          00000000   .text + 26
00000034  00000122 R_AVR_PORT6       00000000   .text + 27
00000036  00000123 R_AVR_PORT5       00000000   .text + 0
00000038  00000115 R_AVR_6_ADIW      00000000   .text + 26

relocated by avr-ld:

$ avr-objdump -d basic-avr  

basic-avr:     file format elf32-avr


Disassembly of section .text:

00000000 <__ctors_end>:
   0:   0e 94 13 00     call    0x26    ; 0x26 <foo>
   4:   06 e2           ldi     r16, 0x26       ; 38
   6:   10 e0           ldi     r17, 0x00       ; 0
   8:   20 e0           ldi     r18, 0x00       ; 0
   a:   30 e0           ldi     r19, 0x00       ; 0
   c:   46 e2           ldi     r20, 0x26       ; 38
   e:   5a ed           ldi     r21, 0xDA       ; 218
  10:   6f ef           ldi     r22, 0xFF       ; 255
  12:   7f ef           ldi     r23, 0xFF       ; 255
  14:   8f ef           ldi     r24, 0xFF       ; 255
  16:   93 e1           ldi     r25, 0x13       ; 19
  18:   a0 e0           ldi     r26, 0x00       ; 0
  1a:   b0 e0           ldi     r27, 0x00       ; 0
  1c:   cd ee           ldi     r28, 0xED       ; 237
  1e:   df ef           ldi     r29, 0xFF       ; 255
  20:   ef ef           ldi     r30, 0xFF       ; 255
  22:   09 f4           brne    .+2             ; 0x26 <foo>
  24:   00 c0           rjmp    .+0             ; 0x26 <foo>

00000026 <foo>:
  26:   2a 80           ldd     r2, Y+2 ; 0x02
  28:   08 80           ld      r0, Y
  2a:   94 84           ldd     r9, Z+12        ; 0x0c
  2c:   76 8c           ldd     r7, Z+30        ; 0x1e
  2e:   96 a0           ldd     r9, Z+38        ; 0x26
  30:   00 91 26 00     lds     r16, 0x0026     ; 0x800026 <__EEPROM_REGION_LENGTH__+0x7f0026>
  34:   47 b5           in      r20, 0x27       ; 39
  36:   00 9a           sbi     0x00, 0 ; 0
  38:   b6 96           adiw    r30, 0x26       ; 38

equals to LLD:

$ avr-objdump -d basic-avr-lld 

basic-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000000 <main>:
   0:   0e 94 13 00     call    0x26    ; 0x26 <foo>
   4:   06 e2           ldi     r16, 0x26       ; 38
   6:   10 e0           ldi     r17, 0x00       ; 0
   8:   20 e0           ldi     r18, 0x00       ; 0
   a:   30 e0           ldi     r19, 0x00       ; 0
   c:   46 e2           ldi     r20, 0x26       ; 38
   e:   5a ed           ldi     r21, 0xDA       ; 218
  10:   6f ef           ldi     r22, 0xFF       ; 255
  12:   7f ef           ldi     r23, 0xFF       ; 255
  14:   8f ef           ldi     r24, 0xFF       ; 255
  16:   93 e1           ldi     r25, 0x13       ; 19
  18:   a0 e0           ldi     r26, 0x00       ; 0
  1a:   b0 e0           ldi     r27, 0x00       ; 0
  1c:   cd ee           ldi     r28, 0xED       ; 237
  1e:   df ef           ldi     r29, 0xFF       ; 255
  20:   ef ef           ldi     r30, 0xFF       ; 255
  22:   09 f4           brne    .+2             ; 0x26 <foo>
  24:   00 c0           rjmp    .+0             ; 0x26 <foo>

00000026 <foo>:
  26:   2a 80           ldd     r2, Y+2 ; 0x02
  28:   08 80           ld      r0, Y
  2a:   94 84           ldd     r9, Z+12        ; 0x0c
  2c:   76 8c           ldd     r7, Z+30        ; 0x1e
  2e:   96 a0           ldd     r9, Z+38        ; 0x26
  30:   00 91 26 00     lds     r16, 0x0026     ; 0x800026 <foo+0x800000>
  34:   47 b5           in      r20, 0x27       ; 39
  36:   00 9a           sbi     0x00, 0 ; 0
  38:   b6 96           adiw    r30, 0x26       ; 38

Regards,
Leslie Zhai

[xiangzhai]

ELF/Arch/AVR.cpp

110

Could you give me the Calculation method for LO8? I follow the binutils's bfd/elf32-avr.c

[xiangzhai]

Add R_AVR_LO8_LDI_GS and R_AVR_HI8_LDI_GS testcase:

$ avr-readelf -r basic-avr.o 

...
00000034  00000118 R_AVR_LO8_LDI_GS  00000000   .text + 26
00000036  00000119 R_AVR_HI8_LDI_GS  00000000   .text + 26
...

relocated by avr-ld:

$ avr-objdump -d basic-avr   

basic-avr:     file format elf32-avr


Disassembly of section .text:

00000000 <__ctors_end>:
   0:   0e 94 13 00     call    0x26    ; 0x26 <foo>
   4:   06 e2           ldi     r16, 0x26       ; 38
   6:   10 e0           ldi     r17, 0x00       ; 0
   8:   20 e0           ldi     r18, 0x00       ; 0
   a:   30 e0           ldi     r19, 0x00       ; 0
   c:   46 e2           ldi     r20, 0x26       ; 38
   e:   5a ed           ldi     r21, 0xDA       ; 218
  10:   6f ef           ldi     r22, 0xFF       ; 255
  12:   7f ef           ldi     r23, 0xFF       ; 255
  14:   8f ef           ldi     r24, 0xFF       ; 255
  16:   93 e1           ldi     r25, 0x13       ; 19
  18:   a0 e0           ldi     r26, 0x00       ; 0
  1a:   b0 e0           ldi     r27, 0x00       ; 0
  1c:   cd ee           ldi     r28, 0xED       ; 237
  1e:   df ef           ldi     r29, 0xFF       ; 255
  20:   ef ef           ldi     r30, 0xFF       ; 255
  22:   09 f4           brne    .+2             ; 0x26 <foo>
  24:   00 c0           rjmp    .+0             ; 0x26 <foo>

00000026 <foo>:
  26:   2a 80           ldd     r2, Y+2 ; 0x02
  28:   08 80           ld      r0, Y
  2a:   94 84           ldd     r9, Z+12        ; 0x0c
  2c:   76 8c           ldd     r7, Z+30        ; 0x1e
  2e:   96 a0           ldd     r9, Z+38        ; 0x26
  30:   00 91 26 00     lds     r16, 0x0026     ; 0x800026 <__EEPROM_REGION_LENGTH__+0x7f0026>
  34:   13 e1           ldi     r17, 0x13       ; 19
  36:   20 e0           ldi     r18, 0x00       ; 0
  38:   47 b5           in      r20, 0x27       ; 39
  3a:   00 9a           sbi     0x00, 0 ; 0
  3c:   b6 96           adiw    r30, 0x26       ; 38

it equals to LLD:

$ avr-objdump -d basic-avr-lld 

basic-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000000 <main>:
   0:   0e 94 13 00     call    0x26    ; 0x26 <foo>
   4:   06 e2           ldi     r16, 0x26       ; 38
   6:   10 e0           ldi     r17, 0x00       ; 0
   8:   20 e0           ldi     r18, 0x00       ; 0
   a:   30 e0           ldi     r19, 0x00       ; 0
   c:   46 e2           ldi     r20, 0x26       ; 38
   e:   5a ed           ldi     r21, 0xDA       ; 218
  10:   6f ef           ldi     r22, 0xFF       ; 255
  12:   7f ef           ldi     r23, 0xFF       ; 255
  14:   8f ef           ldi     r24, 0xFF       ; 255
  16:   93 e1           ldi     r25, 0x13       ; 19
  18:   a0 e0           ldi     r26, 0x00       ; 0
  1a:   b0 e0           ldi     r27, 0x00       ; 0
  1c:   cd ee           ldi     r28, 0xED       ; 237
  1e:   df ef           ldi     r29, 0xFF       ; 255
  20:   ef ef           ldi     r30, 0xFF       ; 255
  22:   09 f4           brne    .+2             ; 0x26 <foo>
  24:   00 c0           rjmp    .+0             ; 0x26 <foo>

00000026 <foo>:
  26:   2a 80           ldd     r2, Y+2 ; 0x02
  28:   08 80           ld      r0, Y
  2a:   94 84           ldd     r9, Z+12        ; 0x0c
  2c:   76 8c           ldd     r7, Z+30        ; 0x1e
  2e:   96 a0           ldd     r9, Z+38        ; 0x26
  30:   00 91 26 00     lds     r16, 0x0026     ; 0x800026 <foo+0x800000>
  34:   13 e1           ldi     r17, 0x13       ; 19
  36:   20 e0           ldi     r18, 0x00       ; 0
  38:   47 b5           in      r20, 0x27       ; 39
  3a:   00 9a           sbi     0x00, 0 ; 0
  3c:   b6 96           adiw    r30, 0x26       ; 38

[xiangzhai]

Add testcase for R_AVR_LDS_STS_16.

$ avr-readelf -r tiny-avr.o 

Relocation section '.rela.text' at offset 0xa4 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000121 R_AVR_LDS_STS_16  00000000   .text + 2
00000002  00000103 R_AVR_13_PCREL    00000000   .text + 2

only for AVR Tiny family:

$ avr-readelf -h tiny-avr.o 
ELF Header:
  Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00 
  Class:                             ELF32
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              REL (Relocatable file)
  Machine:                           Atmel AVR 8-bit microcontroller
  Version:                           0x1
  Entry point address:               0x0
  Start of program headers:          0 (bytes into file)
  Start of section headers:          240 (bytes into file)
  Flags:                             0xe4, avr:100, link-relax
  Size of this header:               52 (bytes)
  Size of program headers:           0 (bytes)
  Number of program headers:         0
  Size of section headers:           40 (bytes)
  Number of section headers:         8
  Section header string table index: 5

LLD:

$ avr-objdump -d tiny-avr-lld 

tiny-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000040 <main>:
  40:   12 a1           ldd     r17, Z+34       ; 0x22

00000042 <foo>:
  42:   ff cf           rjmp    .-2             ; 0x42 <foo>

[ruiu]

This patch still seems odd. It has an unused variable and uses templates where you don't need to use that.

The test does not test the feature enough because, as I wrote, foo's high address is zero.

Please fix all these oddities.

[xiangzhai]

Dear Rui,

I fixed the issue as you point out.

$ avr-gcc -mmcu=atmega328p -o basic-avr.o -c basic-avr.s
$ avr-readelf -r basic-avr.o

Relocation section '.rela.text' at offset 0xe4 contains 30 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000112 R_AVR_CALL        00000000   .text + 2c
00000004  00000105 R_AVR_16_PM       00000000   .text + 2c
00000006  0000011a R_AVR_8           00000000   .text + 2c
00000007  0000011b R_AVR_8_LO8       00000000   .text + 2c
00000008  0000011c R_AVR_8_HI8       00000000   .text + 2c
00000009  0000011d R_AVR_8_HLO8      00000000   .text + 2c
0000000a  00000106 R_AVR_LO8_LDI     00000000   .text + 2c
0000000c  00000107 R_AVR_HI8_LDI     00000000   .text + 2c
0000000e  00000108 R_AVR_HH8_LDI     00000000   .text + 2c
00000010  00000116 R_AVR_MS8_LDI     00000000   .text + 2c
00000012  00000113 R_AVR_LDI         00000000   .text + 2c
00000014  00000109 R_AVR_LO8_LDI_NEG 00000000   .text + 2c
00000016  0000010a R_AVR_HI8_LDI_NEG 00000000   .text + 2c
00000018  0000010b R_AVR_HH8_LDI_NEG 00000000   .text + 2c
0000001a  00000117 R_AVR_MS8_LDI_NEG 00000000   .text + 2c
0000001c  0000010c R_AVR_LO8_LDI_PM  00000000   .text + 2c
0000001e  0000010d R_AVR_HI8_LDI_PM  00000000   .text + 2c
00000020  0000010e R_AVR_HH8_LDI_PM  00000000   .text + 2c
00000022  0000010f R_AVR_LO8_LDI_PM_ 00000000   .text + 2c
00000024  00000110 R_AVR_HI8_LDI_PM_ 00000000   .text + 2c
00000026  00000111 R_AVR_HH8_LDI_PM_ 00000000   .text + 2c
00000028  00000102 R_AVR_7_PCREL     00000000   .text + 2c
0000002a  00000103 R_AVR_13_PCREL    00000000   .text + 2c
00000034  00000114 R_AVR_6           00000000   .text + 2c
00000038  00000104 R_AVR_16          00000000   .text + 2c
0000003a  00000118 R_AVR_LO8_LDI_GS  00000000   .text + 2c
0000003c  00000119 R_AVR_HI8_LDI_GS  00000000   .text + 2c
0000003e  00000122 R_AVR_PORT6       00000000   .text + 2d
00000040  00000123 R_AVR_PORT5       00000000   .text + 0
00000042  00000115 R_AVR_6_ADIW      00000000   .text + 2c

Link high address *not* zero:

`
$ avr-ld -o basic-avr basic-avr.o -Ttext=6
$ avr-objdump -d basic-avr  

basic-avr:     file format elf32-avr


Disassembly of section .text:

00000006 <__ctors_end>:
   6:   0e 94 19 00     call    0x32    ; 0x32 <foo>
   a:   19 00           .word   0x0019  ; ????
   c:   32 32           cpi     r19, 0x22       ; 34
   e:   00 00           nop
  10:   02 e3           ldi     r16, 0x32       ; 50
  12:   10 e0           ldi     r17, 0x00       ; 0
  14:   20 e0           ldi     r18, 0x00       ; 0
  16:   30 e0           ldi     r19, 0x00       ; 0
  18:   42 e3           ldi     r20, 0x32       ; 50
  1a:   5e ec           ldi     r21, 0xCE       ; 206
  1c:   6f ef           ldi     r22, 0xFF       ; 255
  1e:   7f ef           ldi     r23, 0xFF       ; 255
  20:   8f ef           ldi     r24, 0xFF       ; 255
  22:   99 e1           ldi     r25, 0x19       ; 25
  24:   a0 e0           ldi     r26, 0x00       ; 0
  26:   b0 e0           ldi     r27, 0x00       ; 0
  28:   c7 ee           ldi     r28, 0xE7       ; 231
  2a:   df ef           ldi     r29, 0xFF       ; 255
  2c:   ef ef           ldi     r30, 0xFF       ; 255
  2e:   09 f4           brne    .+2             ; 0x32 <foo>
  30:   00 c0           rjmp    .+0             ; 0x32 <foo>

00000032 <foo>:
  32:   2a 80           ldd     r2, Y+2 ; 0x02
  34:   08 80           ld      r0, Y
  36:   94 84           ldd     r9, Z+12        ; 0x0c
  38:   76 8c           ldd     r7, Z+30        ; 0x1e
  3a:   92 a8           ldd     r9, Z+50        ; 0x32
  3c:   00 91 32 00     lds     r16, 0x0032     ; 0x800032 <__EEPROM_REGION_LENGTH__+0x7f0032>
  40:   29 e1           ldi     r18, 0x19       ; 25
  42:   30 e0           ldi     r19, 0x00       ; 0
  44:   43 b7           in      r20, 0x33       ; 51
  46:   30 9a           sbi     0x06, 0 ; 6
  48:   f2 96           adiw    r30, 0x32       ; 50

it equals to LLD:

$ llvm/build/bin/ld.lld -o basic-avr-lld basic-avr.o -Ttext=6
$ avr-objdump -d basic-avr-lld           

basic-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000006 <main>:
   6:   0e 94 19 00     call    0x32    ; 0x32 <foo>
   a:   19 00           .word   0x0019  ; ????
   c:   32 32           cpi     r19, 0x22       ; 34
   e:   00 00           nop
  10:   02 e3           ldi     r16, 0x32       ; 50
  12:   10 e0           ldi     r17, 0x00       ; 0
  14:   20 e0           ldi     r18, 0x00       ; 0
  16:   30 e0           ldi     r19, 0x00       ; 0
  18:   42 e3           ldi     r20, 0x32       ; 50
  1a:   5e ec           ldi     r21, 0xCE       ; 206
  1c:   6f ef           ldi     r22, 0xFF       ; 255
  1e:   7f ef           ldi     r23, 0xFF       ; 255
  20:   8f ef           ldi     r24, 0xFF       ; 255
  22:   99 e1           ldi     r25, 0x19       ; 25
  24:   a0 e0           ldi     r26, 0x00       ; 0
  26:   b0 e0           ldi     r27, 0x00       ; 0
  28:   c7 ee           ldi     r28, 0xE7       ; 231
  2a:   df ef           ldi     r29, 0xFF       ; 255
  2c:   ef ef           ldi     r30, 0xFF       ; 255
  2e:   09 f4           brne    .+2             ; 0x32 <foo>
  30:   00 c0           rjmp    .+0             ; 0x32 <foo>

00000032 <foo>:
  32:   2a 80           ldd     r2, Y+2 ; 0x02
  34:   08 80           ld      r0, Y
  36:   94 84           ldd     r9, Z+12        ; 0x0c
  38:   76 8c           ldd     r7, Z+30        ; 0x1e
  3a:   92 a8           ldd     r9, Z+50        ; 0x32
  3c:   00 91 32 00     lds     r16, 0x0032     ; 0x800032 <foo+0x800000>
  40:   29 e1           ldi     r18, 0x19       ; 25
  42:   30 e0           ldi     r19, 0x00       ; 0
  44:   43 b7           in      r20, 0x33       ; 51
  46:   30 9a           sbi     0x06, 0 ; 6
  48:   f2 96           adiw    r30, 0x32       ; 50

Regards,
Leslie Zhai

[xiangzhai]

Add testcase for tinyAVR about R_AVR_LDS_STS_16:

$ avr-gcc -mmcu=attiny40 -o tiny-avr.o -c tiny-avr.s
$ avr-readelf -r tiny-avr.o

Relocation section '.rela.text' at offset 0xa4 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000121 R_AVR_LDS_STS_16  00000000   .text + 2
00000002  00000103 R_AVR_13_PCREL    00000000   .text + 2

Relocated by avr-ld:

Memory access is limited to the address range 0x40...0xbf. so use 0x40 as high address:

$ avr-ld --no-warn-mismatch -o tiny-avr tiny-avr.o -Ttext=40
$ avr-objdump -d tiny-avr

tiny-avr:     file format elf32-avr


Disassembly of section .text:

00000040 <__ctors_end>:
  40:   12 a1           ldd     r17, Z+34       ; 0x22

00000042 <foo>:
  42:   ff cf           rjmp    .-2             ; 0x42 <foo>

It is equals to LLD:

$ llvm/build/bin/ld.lld -o tiny-avr-lld tiny-avr.o -Ttext=40
$ avr-objdump -d tiny-avr-lld 

tiny-avr-lld:     file format elf32-avr


Disassembly of section .text:

00000040 <main>:
  40:   12 a1           ldd     r17, Z+34       ; 0x22

00000042 <foo>:
  42:   ff cf           rjmp    .-2             ; 0x42 <foo>

[xiangzhai]

Dear Rui,

I have removed all templates and tested with several different high address not ZERO, the same Disassmebly compared with ld.bfd, please review my patch, thanks a lot!

Regards,
Leslie Zhai

[ruiu]

There's still odd pieces of code there. For example, I don't know about how R_AVR_8_LO8 should be handled, but I can say that this piece of code is odd

case R_AVR_8_LO8:
  write16le(Loc, SRel & 0xffffff);

because masking all but the least significant 24 bits before writing the least significant 16 bit doesn't make sense.

As I wrote before, could you please review your code yourself more carefully before sending it for another round of code review?

[ruiu]

Well, I found that this patch contains too much duplicate code in GNU bfd (e.g. (x & 0xfc07) | (((srel >> 1) << 3) & 0x3f8), which is odd by the way because of redundant right shift before left shift). These pieces of code are exactly the same including variable names. Please don't do that unless you know what you are doing (if that's the case, please mention it in the commit message). In general, you cannot reuse GPL'ed code to lld. You can use bfd linker to understand the AVR relocations, but your patch needs be of your own work. I think you need to rewrite the patch entirely.

[MaskRay]

@xiangzhai Is this patch superseded by D78741?

ELF/Arch/AVR.cpp

 #include "InputFiles.h"
 #include "Symbols.h"
 #include "Target.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/Endian.h"
 
 using namespace llvm;
 using namespace llvm::object;
+using namespace llvm::support;
 using namespace llvm::support::endian;
 using namespace llvm::ELF;
 using namespace lld;
 using namespace lld::elf;
 
 namespace {
+uint16_t caculateForLDI(uint16_t X, uint16_t SRel) {
+  return (X & 0xf0f0) | (SRel & 0xf) | ((SRel << 4) & 0xf00);
+}
+
 class AVR final : public TargetInfo {
 public:
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S, const InputFile &File,
                      const uint8_t *Loc) const override;
   void relocateOne(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
 };
 } // namespace
 
 RelExpr AVR::getRelExpr(uint32_t Type, const SymbolBody &S,
                         const InputFile &File, const uint8_t *Loc) const {
   switch (Type) {
+  case R_AVR_7_PCREL:
+  case R_AVR_13_PCREL:
+    return R_PC;
+  case R_AVR_LO8_LDI:
+  case R_AVR_LDI:
+  case R_AVR_6:
+  case R_AVR_6_ADIW:
+  case R_AVR_HI8_LDI:
+  case R_AVR_HH8_LDI:
+  case R_AVR_MS8_LDI:
+  case R_AVR_LO8_LDI_NEG:
+  case R_AVR_HI8_LDI_NEG:
+  case R_AVR_HH8_LDI_NEG:
+  case R_AVR_MS8_LDI_NEG:
+  case R_AVR_LO8_LDI_GS:
+  case R_AVR_LO8_LDI_PM:
+  case R_AVR_HH8_LDI_PM:
+  case R_AVR_LO8_LDI_PM_NEG:
+  case R_AVR_HI8_LDI_PM_NEG:
+  case R_AVR_HH8_LDI_PM_NEG:
   case R_AVR_CALL:
+  case R_AVR_16:
+  case R_AVR_16_PM:
+  case R_AVR_LDS_STS_16:
+  case R_AVR_PORT6:
+  case R_AVR_PORT5:
     return R_ABS;
   default:
     error(toString(&File) + ": unknown relocation type: " + toString(Type));
     return R_HINT;
   }
 }
 
 void AVR::relocateOne(uint8_t *Loc, uint32_t Type, uint64_t Val) const {
+  uint16_t SRel = Val;
+  uint16_t X = read16le(Loc);
   switch (Type) {
-  case R_AVR_CALL: {
-    uint16_t Hi = Val >> 17;
-    uint16_t Lo = Val >> 1;
-    write16le(Loc, read16le(Loc) | ((Hi >> 1) << 4) | (Hi & 1));
-    write16le(Loc + 2, Lo);
+  case R_AVR_7_PCREL:
+    SRel -= 2;
+    X = (X & 0xfc07) | (((SRel >> 1) << 3) & 0x3f8);
+    write16le(Loc, X);
+    break;
+  case R_AVR_13_PCREL:
+    SRel -= 2;
+    SRel >>= 1;
+    X = (X & 0xf000) | (SRel & 0xfff);
+    write16le(Loc, X);
+    break;
+  case R_AVR_LO8_LDI:
+  case R_AVR_LDI:
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_6:
+    X = (X & 0xd3f8) | ((SRel & 7) | ((SRel & (3 << 3)) << 7)
+                     | ((SRel & (1 << 5)) << 8));
+    write16le(Loc, X);
+    break;
+  case R_AVR_6_ADIW:
+    X = (X & 0xff30) | (SRel & 0xf) | ((SRel & 0x30) << 2);
+    write16le(Loc, X);
+    break;
+  case R_AVR_HI8_LDI:
+    SRel = (SRel >> 8) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_HH8_LDI:
+    SRel = (SRel >> 16) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_MS8_LDI:
+    SRel = (SRel >> 24) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_LO8_LDI_NEG:
+    write16le(Loc, caculateForLDI(X, -SRel));
+    break;
+  case R_AVR_HI8_LDI_NEG:
+    SRel = ((-SRel) >> 8) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_HH8_LDI_NEG:
+    SRel = ((-SRel) >> 16) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_MS8_LDI_NEG:
+    SRel = ((-SRel) >> 24) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_LO8_LDI_GS:
+  case R_AVR_LO8_LDI_PM:
+    SRel = (SRel >> 9) & 0xff;;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_HH8_LDI_PM:
+    SRel = (SRel >> 17) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_LO8_LDI_PM_NEG:
+    SRel = (-SRel) >> 1;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_HI8_LDI_PM_NEG:
+    SRel = ((-SRel) >> 9) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_HH8_LDI_PM_NEG:
+    SRel = ((-SRel) >> 17) & 0xff;
+    write16le(Loc, caculateForLDI(X, SRel));
+    break;
+  case R_AVR_CALL:
+    SRel = SRel >> 1;
+    X |= ((SRel & 0x10000) | ((SRel << 3) & 0x1f00000)) >> 16;
+    write16le(Loc, X);
+    write16le(Loc + 2, SRel & 0xffff);
+    break;
+  case R_AVR_16:
+  case R_AVR_16_PM:
+    write16le(Loc, (SRel >> 1) & 0x00ffff);
+    break;
+  case R_AVR_LDS_STS_16:
+    SRel = SRel & 0x7f;
+    X |= (SRel & 0x0f) | ((SRel & 0x30) << 5) | ((SRel & 0x40) << 2);
+    write16le(Loc, X);
+    break;
+  case R_AVR_PORT6:
+    X = (X & 0xf9f0) | ((SRel & 0x30) << 5) | (SRel & 0x0f);
+    write16le(Loc, X);
+    break;
+  case R_AVR_PORT5:
+    X = (X & 0xff07) | ((SRel & 0x1f) << 3);
+    write16le(Loc, X);
     break;
-  }
   default:
     error(getErrorLocation(Loc) + "unrecognized reloc " + toString(Type));
   }
 }
 
 TargetInfo *elf::getAVRTargetInfo() {
   static AVR Target;
   return &Target;
 }

test/ELF/basic-avr.s

 # REQUIRES: avr
 # RUN: llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p %s -o %t.o
 # RUN: ld.lld %t.o -o %t.exe -Ttext=0
 # RUN: llvm-objdump -d %t.exe | FileCheck %s
 
 main:
-  call foo
+  ldi r17, lo8(foo)
+  ldi r18, hi8(bar)
+  ldi r19, hh8(foo)
+  ldi r20, foo
+  brne next
+  rjmp next
+  adiw r24, bar
+next:
+  jmp next
 foo:
   jmp foo
+bar:
+  jmp bar
 
 # CHECK:      main:
-# CHECK-NEXT:   0: 0e 94 02 00 <unknown>
+# CHECK-NEXT:   0: 12 e1 <unknown>
+# CHECK-NEXT:   2: 20 e0 <unknown>
+# CHECK-NEXT:   4: 30 e0 <unknown>
+# CHECK-NEXT:   6: 42 e1 <unknown>
+# CHECK-NEXT:   8: 11 f4 <unknown>
+# CHECK-NEXT:   a: 01 c0 <unknown>
+# CHECK-NEXT:   c: 46 96 <unknown>
+# CHECK:      next:
+# CHECK-NEXT:   e: 0c 94 07 00 <unknown>
 # CHECK:      foo:
-# CHECK-NEXT:   4: 0c 94 02 00 <unknown>
+# CHECK-NEXT:  12: 0c 94 09 00 <unknown>
+# CHECK:      bar:
+# CHECK-NEXT:  16: 0c 94 0b 00 <unknown>