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

[Jinyang He via Phabricator via llvm-commits]

MQ-mengqing added inline comments.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:430
+  case R_LARCH_32:
+  case R_LARCH_32_PCREL:
+    write32le(loc, val);
----------------
For R_LARCH_32_PCREL, it is better to check whether overflow.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:470
+  case R_LARCH_TLS_IE_LO12: {
+    // TODO: checkInt(loc, SignExtend64(hi, bits) >> 12, 20, rel);
+    write32le(loc, setK12(read32le(loc), extractBits(val, 11, 0)));
----------------
The R_LARCH_{GOT_LO12, LE_LO12, IE_LO12} are more like R_LARCH_ABS_LO12.


================
Comment at: lld/ELF/Arch/LoongArch.cpp:487
+  case R_LARCH_TLS_GD_HI20: {
+    // TODO: checkInt(loc, SignExtend64(hi, bits) >> 12, 20, rel);
+    write32le(loc, setJ20(read32le(loc), extractBits(val, 31, 12)));
----------------
Why so many TODOs here?



================
Comment at: lld/ELF/InputSection.cpp:646
   case R_RELAX_TLS_GD_TO_IE_ABS:
+    // XXX Why???
+    if (config->emachine == EM_LOONGARCH && sym.isTls()) {
----------------
Due to loongarch override the GOT(64)_(PC_){LO12, LO20, HI12} in the LD/GD case, the logic is complex.
I found it is hardly to get the right value form the value of TlsIndex or  the value of GlobalDynamic, when I ported lld to llvm-12 in our internal repository.
And I tried to use others RelExpr to get them correctly. For GD and LD we should get the GlobalDynamic and for IE we should get the TlsIndex.
If you do not distinguish them in LoongArch::getRelExpr, it might wrong, or I missed things in llvm-16.
Is there no tls test in this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138135