[lld] [lld-macho] Fix branch extension thunk estimation logic (PR #120529)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 7 14:20:46 PST 2025


================
@@ -184,15 +184,45 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const {
     InputSection *isec = inputs[i];
     isecEnd = alignToPowerOf2(isecEnd, isec->align) + isec->getSize();
   }
+
+  // Tally up any thunks that have already been placed that have VA higher than
+  // inputs[callIdx]. First, find the index of the first thunk that is beyond
+  // the current inputs[callIdx].
+  auto itPostcallIdxThunks =
+      llvm::partition_point(thunks, [isecVA](const ConcatInputSection *t) {
+        return t->getVA() <= isecVA;
+      });
+  uint64_t existingForwardThunks = thunks.end() - itPostcallIdxThunks;
+
----------------
alx32 wrote:

> One should be able to figure out this value during the previous iteration and avoid the partition_point call.

This is true, but i think this might be less performant. The for loop iterates all thunks which there can be many of (100k+). That means adding a branch that executes 100k times.

`maxPotentialThunks` is log(n) complexity so it for ~100k input thunks it will have <20 iterations and a few hundred instructions executed. 

Generally we favor more performant code for LLD - even if here it probably won't really be measurable.
Or do we want this for easier code readability / understandability ?

> It also looks like we might be double counting in `maxPotentialThunks` and `existingForwardThunks`, but there might be some not-easy-to-see dependency in how thunkMap is used and when estimateStubsInRangeVA is called that avoids that situation

This is basically the issue to be fixed. `maxPotentialThunks` is intended and appears to contain all the possible thunks and adding in `existingForwardThunks` would appear to be double counting . But that's exactly the original bug - `maxPotentialThunks` does not include all the thunks so we need to count all the thunks by adding `existingForwardThunks`.

https://github.com/llvm/llvm-project/pull/120529


More information about the llvm-commits mailing list