[PATCH] D48929: [ELF] Update addends in non-allocatable sections for REL targets when creating a relocatable output.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputSection.cpp:732
 void InputSectionBase::relocate(uint8_t *Buf, uint8_t *BufEnd) {
   if (Flags & SHF_ALLOC) {
     relocateAlloc(Buf, BufEnd);
----------------
Should instead of all other changes in this patch this line just be a 

`(Flags & SHF_ALLOC || Config->Relocatable)` ?

With the corresponding renaming/commenting of things may be.

We have `relocateAlloc`/`relocateNonAlloc` splitted mostly for speedup of
processing debug relocations I think. 

I do not feel it is very critical for -r. It seems better to reuse the existent
code than to add new.

Rui, what do you think?


================
Comment at: ELF/InputSection.cpp:834
+  }
+}
+
----------------
This function is a bit overloaded with excessive variables.
It could be shorter:

```
void InputSectionBase::relocateNonAllocForRelocatable(uint8_t *Buf, uint8_t *BufEnd) {
  for (const Relocation &Rel : Relocations) {
    // InputSection::copyRelocations() adds only R_ABS relocations.
    assert(Rel.Expr == R_ABS);
    uint8_t *BufLoc = Rel.Offset + cast<InputSection>(this)->OutSecOff + Offset;
    uint64_t TargetVA =
        SignExtend64(Rel.Sym->getVA(Rel.Addend), Config->Wordsize * 8);
    Target->relocateOne(BufLoc, Rel.Type, TargetVA);
  }
}
```

And then probably inlined. (If will be decided to not to reuse the code).


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48929