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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:111
+  //
+  // With dest=0x900'10000'abcde'f00 and pc=0x800'20345'ba987'654, we want:
+  //
----------------
This example doesn't trigger the interesting case `signLo12 != signHi20`. We need a different example.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:125
+  if (signLo12 && !signHi20)
+      result -= 0x100000000;
+  else if (!signLo12 && signHi20)
----------------
excessively indented


================
Comment at: lld/ELF/Arch/LoongArch.cpp:219
+static bool inputFileHasCode(const InputFile *f) {
+  if (!f)
+    return false;
----------------
Delete nullness check.

We probably should change the parameter to a const reference.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:222
+
+  for (auto *sec : f->getSections()) {
+    if (sec && sec->flags & SHF_EXECINSTR)
----------------
drop braces


================
Comment at: lld/ELF/InputSection.cpp:655
+      return in.got->getGlobalDynAddr(sym) + a;
+    // Fallthrough; relocate like R_GOT.
+    return sym.getGotVA() + a;
----------------
Delete this comment. This is not fallthrough, which usually means `[[fallthrough]]`.


================
Comment at: lld/ELF/Relocations.h:110
+  // In addition to having page-aligned semantics, LoongArch GOT relocs are
+  // also reused for TLS, making the semantics differ from everyone else.
+  R_LOONGARCH_GOT,
----------------
... from other architectures.


================
Comment at: lld/test/ELF/loongarch-branch-b16.s:22
+# RUN: not ld.lld %t.la64.o --defsym foo=_start+0x20000 --defsym bar=_start+4-0x20004 -o /dev/null 2>&1 | FileCheck --check-prefix=ERROR-RANGE %s
+# ERROR-RANGE: relocation R_LARCH_B16 out of range: 131072 is not in [-131072, 131071]; references 'foo'
+# ERROR-RANGE: relocation R_LARCH_B16 out of range: -131076 is not in [-131072, 131071]; references 'bar'
----------------
For error messages, we check the `error: ` prefix.

Fix this elsewhere.


================
Comment at: lld/test/ELF/loongarch-branch-b16.s:23
+# ERROR-RANGE: relocation R_LARCH_B16 out of range: 131072 is not in [-131072, 131071]; references 'foo'
+# ERROR-RANGE: relocation R_LARCH_B16 out of range: -131076 is not in [-131072, 131071]; references 'bar'
+
----------------
Add `-NEXT` whenever applicable.


================
Comment at: lld/test/ELF/loongarch-branch-b21.s:22
+# RUN: not ld.lld %t.la64.o --defsym foo=_start+0x400000 --defsym bar=_start+4-0x400004 -o /dev/null 2>&1 | FileCheck --check-prefix=ERROR-RANGE %s
+# ERROR-RANGE: relocation R_LARCH_B21 out of range: 4194304 is not in [-4194304, 4194303]; references 'foo'
+# ERROR-RANGE: relocation R_LARCH_B21 out of range: -4194308 is not in [-4194304, 4194303]; references 'bar'
----------------
For error messages, we check the error:  prefix.


================
Comment at: lld/test/ELF/loongarch-branch-b26.s:1
+# REQUIRES: loongarch
+
----------------
I think it helps readability if loongarch-branch-b26.s is merged into loongarch-branch-b21.s.


================
Comment at: lld/test/ELF/loongarch-branch-b26.s:27
+# RUN: not ld.lld %t.la64.o --defsym foo=_start+1 --defsym bar=_start-1 -o /dev/null 2>&1 | FileCheck --check-prefix=ERROR-ALIGN-1 %s
+# ERROR-ALIGN-1: improper alignment for relocation R_LARCH_B26: 0xFFFFFFFFFFFFFFFB is not aligned to 4 bytes
+
----------------
For error messages, we check the error:  prefix.


================
Comment at: lld/test/ELF/loongarch-interlink.test:76
+_start:
+la $a0, blob
+ld.b $a0, $a0, 0
----------------
Indent instructions


================
Comment at: lld/test/ELF/loongarch-plt.s:83
+## 64-bit: &.got.plt[bar]-. = 0x400e0-0x20040 = 4096*32+160
+# DIS:   20040: pcaddu12i $t3, 32
+# DIS32-NEXT:   ld.w    $t3, $t3, 48
----------------
The column number of 20040: should not be less than the column number of the colon in `DIS32-NEXT:`.
We may need more indentation for instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138135