[PATCH] D138135: [lld][ELF] Support LoongArch

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added inline comments.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:341
+  case R_LARCH_NONE:
+  case R_LARCH_MARK_LA:
+  case R_LARCH_MARK_PCREL:
----------------
MaskRay wrote:
> What are R_LARCH_MARK_LA and R_LARCH_MARK_PCREL? They are untested.
AFAIK they serve as markers for external consumers only: `R_LARCH_MARK_LA` is emitted by binutils for each `la.abs`, and used by (old) Linux and grub to perform their own relocation; `R_LARCH_MARK_PCREL` is unused so far. Are test cases still needed in this case?


================
Comment at: lld/test/ELF/loongarch-pc-aligned.s:15
+#--- case1.t
+SECTIONS {
+ .rodata 0x1000: { *(.rodata) }
----------------
MaskRay wrote:
> Consider `--section-start`
Strangely I get `error: output file too large` for some of the cases (like this one; but not with others). Can't see a reason for this...


================
Comment at: lld/test/ELF/loongarch-tls-gd-and-ie.s:1
+# REQUIRES: loongarch
+
----------------
MaskRay wrote:
> What does loongarch-tls-gd-and-ie.s mean?
It's an edge case discovered by @MQ-mengqing and explained in a comment below. I've renamed the test case and moved the comment to top for better visibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138135