[PATCH] D53905: [ELF] Refactor TLS layout TargetInfo config. NFC.

[James Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: ELF/InputSection.cpp:572-573
+  // target-specific adjustment to produce a thread-pointer-relative offset.
+  switch (Target->TlsLayout) {
+    case TlsLayoutKind::FixedOffset:
+      return Target->TlsTpOffset;
----------------
ruiu wrote:
> I don't think you need to define TlsLayoutKind. I'd just dispatch based on `Config->EMachine` in this function.
That works, and there are certainly other parts of LLD that do things like that, but you could say the same thing about `TlsTpOffset` and `TcbSize` themselves... I personally like to see `Config->EMachine` used as little as possible and put the logic in `ELF/Arch/$ARCH.cpp`, but of course as maintainer you ultimately decide where to draw the line for when to abstract, and it's not exactly a big deal (though does then disregard the original motivation for this patch).

Also, on a separate issue, the case labels (and bodies) should be deindented one level to be flush with the `switch`.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53905