[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