[PATCH] D100818: [lld-macho] Implement branch-range-extension thunks

[Greg McGary via Phabricator via llvm-commits]

gkm marked 28 inline comments as done.
gkm added a comment.

In D100818#2729135 <https://reviews.llvm.org/D100818#2729135>, @tschuett wrote:

> Feel free to ignore me,

I won't ignore you, but I will punt and keep your recommendations in mind for some future time. Thanx!

> but it is an odd dance with the default implementation of populateThunk, which is unreachable.

It now calls `llvm_unreachable()` !



================
Comment at: lld/MachO/MergedOutputSection.cpp:144-146
+  size_t ic = 0;
+  size_t ix = 0;
+  size_t ie = inputs.size();
----------------
int3 wrote:
> I find these kind of hard to read, could we use slightly more verbose names?
s/`ic`/`I`/; s/`ie`/`E`/, by convention I see many places in LLVM code, though not so much in LLD yet.
s/`ix`/`iFinal`/

Let me know if you hate `I` and `E`.


================
Comment at: lld/MachO/MergedOutputSection.cpp:160
+    // Process relocs by ascending address
+    for (Reloc &r : reverse(isec->relocs)) {
+      relocCount++;
----------------
int3 wrote:
> I believe llvm-mc emits relocs in reverse order, but I don't think that's guaranteed anywhere in the format... we should probably sort it ourselves
Before I add code and overhead to sort already-sorted vectors, I'd like to look into this further.


================
Comment at: lld/MachO/MergedOutputSection.cpp:222
+
+  if (config->verbose)
+    warn("thunks for " + parent->name + "," + name +
----------------
int3 wrote:
> LLD-ELF handles `--verbose` by assigning to `errorHandler().verbose`, I think we should do likewise
Done. Note that ELF has no other use of `OPT_verbose`. COFF assigns to `errorHandler().verbose`, and also has `config->verbose` to enable other output.


================
Comment at: lld/MachO/MergedOutputSection.cpp:233-244
+  size_t i = 0, ie = inputs.size();
+  size_t t = 0, te = thunks.size();
+  while (i < ie || t < te) {
+    while (i < ie && (t == te || inputs[i]->getSize() == 0 ||
+                      inputs[i]->outSecOff < thunks[t]->outSecOff)) {
+      inputs[i]->writeTo(buf + inputs[i]->outSecFileOff);
+      i++;
----------------
int3 wrote:
> instead of two sorted arrays, would it be simpler to create a map of regular InputSection to an array of thunks that immediately follow it?
Sounds like more overhead & thus slower.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100818