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

Fri Nov 25 08:40:51 PST 2022

xen0n marked 3 inline comments as done.
xen0n added inline comments.

================
Comment at: lld/ELF/InputSection.cpp:646
   case R_RELAX_TLS_GD_TO_IE_ABS:
+    // XXX Why???
+    if (config->emachine == EM_LOONGARCH && sym.isTls()) {
----------------
xen0n wrote:
> MQ-mengqing wrote:
> > 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?
> Ah. My intuition is unfortunately right, the BFD implementation is hiding too much logic behind innocent-looking relocs... and the logic isn't documented so I can't figure out by only looking at the docs.
> 
> For now it seems more `RelExpr`'s are indeed necessary as you said. I'll go through the code once more and try to come up with some solution in the next revision. Thanks for the explanation.
I saw the dubious code controlling the complex behavior you mentioned before, they even wrote "troublesome" themselves in `bfd/elfnn-loongarch.c`...

I've added a new `RelExpr` `R_LOONGARCH_GOT` to hopefully not touch `R_GOT` behavior as far as possible. Unfortunately this has led to increase of complexity in `Relocations.cpp` but that file is arguably already containing enough of entangled logic that my addition is tolerable...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138135