[PATCH] D63220: [ELF][RISCV] Support GD/LD/IE/LE TLS models

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: ELF/Relocations.cpp:224
+  // If the TLS model can be relaxed to Local-Exec, DTPMOD is constant one.
+  bool IsLocalExec = !Config->Shared && !Sym.IsPreemptible;
+
----------------
MaskRay wrote:
> jrtc27 wrote:
> > The RISC-V behaviour is identical to ARM if I remember correctly, so we should be reusing that code rather than reimplementing it again here.
> R_ARM_TLS_GD32 <-> R_RISCV_TLS_GD_HI20
> R_ARM_TLS_LDM32 <-> R_RISCV_TLS_GD_HI20 with a local symbol .LANCHOR0
> R_ARM_TLS_IE32 <-> R_RISCV_TLS_GOT_HI20
> R_ARM_TLS_LE32 <-> R_RISCV_TPREL_{HI20,LO12_I,LO12_S}
> 
> If this is what you mean :)
> 
> However, there is a big difference: RISC-V doesn't support relaxation. So I have to make sure no R_RELAX_TLS_* code is called.
> 
> Having the two variables is the least intrusive approach I can think of. (I complained the lack of linker relaxation turns out to make lld's generic TLS handling uglier. Here it is :))
> 
> 
Sorry, I think you probably meant we can reuse `handleARMTlsRelocation()` for RISC-V. I have compared it with `handleTlsRelocation()` and find it may be worth merging the two functions. Created D63333 for that idea. With that patch, the Relocations.cpp change in this patch will be trivial.

```
- bool Relax = Config->EMachine != EM_ARM;
+ bool Relax = Config->EMachine != EM_ARM && Config->EMachine != EM_RISCV;
```


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D63220