[PATCH] D31751: [LLD][ELF] Tidy up handleARMTlsRelocation [NFC]

[Peter Smith via Phabricator via llvm-commits]

peter.smith added inline comments.


================
Comment at: ELF/Target.cpp:1842
   case R_ARM_TLS_TPOFF32:
+  case R_ARM_TLS_DTPOFF32:
     write32le(Loc, Val);
----------------
ruiu wrote:
> Why do you need this?
It is so the same relocation code can be used in: 

```
AddTlsReloc(Off + Config->Wordsize, Target->TlsOffsetRel, &Body, NeedDynOff);
```
Without it, it would need to be

```
AddTlsReloc(Off + Config->Wordsize, NeedDynOff ? Target->TlsOffsetRel : R_ARM_ABS32, NeedDynOff);
```
As when we statically come to resolve the R_ARM_TLS_DTPOFF32 we get unknown relocation code.

I thought that the trade off in adding R_ARM_TLS_DTPOFF32 was worth it as it emphasized that the dynamic relocation was just being evaluated at static link time. I'm not hugely fussed about doing it this way.


================
Comment at: ELF/Target.cpp:1845-1847
   case R_ARM_TLS_DTPMOD32:
     write32le(Loc, 1);
     break;
----------------
ruiu wrote:
> This is not a comment to this particular patch, but we need to remove this piece of code and instead create a way to just set "1" to .got without using this type of relocation in an odd way.
These possibilities come to mind:
- Add a function to add arbitrary constants to the GotSection, the module index becomes In<ELFT>::Got->addConstant(Off, 1);
- Add a function to add the executables module index at an offset, In<ELFT>::Got->addModuleIndex(Off)
- Make an ARMGotSection that handles the module index in a similar way to the MipsGotSection

I'd like if we have a solution that both you and Rafael are happy with. I have a weak preference for resolving the Relocation as 1 until there are other cases where we need to write arbitrary constants to the GotSection, but as I say this is a weak preference and I'm happy to change it.




https://reviews.llvm.org/D31751