[PATCH] D50632: [LLD][ELF] - Simplify TLS LD handling code.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Arch/PPC64.cpp:742-743
+  if (Expr == R_RELAX_TLS_LD_TO_LE) {
+    if (Type == R_PPC64_GOT_DTPREL16_HA || Type == R_PPC64_GOT_DTPREL16_LO_DS ||
+        Type == R_PPC64_GOT_DTPREL16_DS || Type == R_PPC64_GOT_DTPREL16_HI)
+      return Expr;
----------------
ruiu wrote:
> The old code doesn't say anything about these relocations, but your new code contains code for these relocations. Where did they come from? What is special about them?
The original code did that:

```
  if (Expr == R_TLSLD_GOT_OFF) {
    // Local-Dynamic relocs can be relaxed to local-exec
    if (!Config->Shared) {
      C.Relocations.push_back({R_RELAX_TLS_LD_TO_LE, Type, Offset, Addend, &Sym});
```

The new code does:

```
if (isRelExprOneOf<R_TLSLD_* ..., R_TLSLD_GOT_OFF>(Expr)) {
    // Local-Dynamic relocs can be relaxed to Local-Exec.
    if (!Config->Shared) {
      C.Relocations.push_back(
          {Target->adjustRelaxExpr(Type, nullptr, R_RELAX_TLS_LD_TO_LE), Type,
           Offset, Addend, &Sym});

```

See, the new code calls `Target->adjustRelaxExpr` before pushing the relocations
(btw, that is consistent with the other places in this method).
`Target->adjustRelaxExpr` should leave some relocations along than
(not convert the expression for them).

(Note, such an approach is not new, for example, we are doing the same on AArch64 I think:
https://github.com/llvm-mirror/lld/blob/master/ELF/Arch/AArch64.cpp#L125)


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

https://reviews.llvm.org/D50632