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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay 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 "
----------------
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.


================
Comment at: lld/test/ELF/avrtiny-reloc.s:1
+; REQUIRES: avr
+; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=attiny10 %s -o %t.o
----------------
`avr-tiny*`

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


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

https://reviews.llvm.org/D139807