[PATCH] D147364: [lld][ELF] Support relocations R_AVR_LO8_LDI_GS/R_AVR_HI8_LDI_GS

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Sorry for my belated review. I finished a long trip and had a very long task queue....



================
Comment at: lld/ELF/Arch/AVR.cpp:113
+    // [0, 0x1fffe].
+    assert((s.getVA() & 1) == 0 && "function address must be aligned to 2.");
+    return s.getVA() >= 0x20000;
----------------
If `s.getVA() & 1` is not rejected by previous checks, we should not have this check.

Don't add a period for messages. See https://llvm.org/docs/CodingStandards.html#error-and-warning-messages


================
Comment at: lld/ELF/Thunks.cpp:271
 
+// AVR thunk
+class AVRThunk : public Thunk {
----------------
Don't copy the `MIPS LA25 thunk` comment. It doesn't provide any value for a reader.

Use a short description.


================
Comment at: lld/ELF/Thunks.cpp:860
+void AVRThunk::writeTo(uint8_t *buf) {
+  write32(buf, 0x940c); // Use the direct jump instruction.
+  target->relocateNoSym(buf, R_AVR_CALL, destination.getVA());
----------------
Just write the instruction form, as an example for other architectures: `// b .+4`


================
Comment at: lld/test/ELF/avr-thunk.s:8
+
+; CHECK:       000110b4 <__AVRThunk_b>:
+; CHECK-NEXT:    jmp   0x20000
----------------
Where is 000110b4 referenced in the CHECK lines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147364