[PATCH] D73254: [ELF] Rename relocateOne() to relocate() and pass `Relocation` to it

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: lld/ELF/Target.h:92
+    relocate(loc, rel, val);
+  }
 
----------------
grimar wrote:
> ```
> Relocation rel{};
> rel.expr = R_NONE;
> rel.type = type;
> ```
> 
> This will initialize the `expr` and `type` fields twice, isn't?
> First to the default values and then to `R_NONE` and `type`.
> Applying relocations is a path that is performance sensible.
> I am not sure how compiler can/will optimize this place, but why don't you just want to use
> `relocate(loc, {R_NONE, type, 0, 0, nullptr}, val);` to avoid double initialization?
> 
The compiler can easily detect that the duplicate initialization can be optimized out. `relocateNoSym` call sites are not sensitive (pltHeader,thunk,errata,... ; these are not frequently called).

`{R_NONE, type, 0, 0, nullptr}` => some members are of the same type. If we made a mistake reordering 2 members, it might not be easy to detect.

The explicit approach does not look very bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73254