[PATCH] D139807: [lld][ELF] Support relocation R_AVR_LDS_STS_16 on AVRTiny devices.

[Ben Shi via Phabricator via llvm-commits]

benshi001 marked 2 inline comments as done.
benshi001 added inline comments.


================
Comment at: lld/ELF/Arch/AVR.cpp:175
+  case R_AVR_LDS_STS_16: {
+    if ((calcEFlags() & EF_AVR_ARCH_MASK) != EF_AVR_ARCH_AVRTINY)
+      error(getErrorLocation(loc) + "relocation 'R_AVR_LDS_STS_16' is only "
----------------
MaskRay wrote:
> benshi001 wrote:
> > 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.
> `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.
Thanks. I have removed this unnecessary check.


================
Comment at: lld/test/ELF/avr-reloc.s:48
+; CHECK-LABEL: section .SIX:
+; CHECK:       std   Y+30, r9
+; CHECK-NEXT:  ldd   r9, Y+30
----------------
Currently the disassembler can correctly decode those instructions.


================
Comment at: lld/test/ELF/avrtiny-reloc.s:1
+; REQUIRES: avr
+; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=attiny10 %s -o %t.o
----------------
MaskRay wrote:
> `avr-tiny*`
> 
> Can `avr-reloc.s` be augmented to test `-mcpu=attiny10`?
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139807/new/

https://reviews.llvm.org/D139807