[PATCH] D108924: [lld/mac] Tweak estimateStubsInRangeVA a bit
Nico Weber via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 30 09:57:17 PDT 2021
thakis created this revision.
thakis added a reviewer: lld-macho.
Herald added a reviewer: gkm.
Herald added a project: lld-macho.
thakis requested review of this revision.
- Move a few variables closer to their uses, remove some completely (no behavior change)
- Add some comments
- Make maxPotentialThunks include calls to stubs. It's possible that an earlier call to a stub late in the stub table will need a thunk, and that inserted thunk could push a stub earlier in the stub table out of range. This is unlikely to happen, but usually there are way fewer stub calls than non-stub calls, so if we're doing a conservative approximation here we might as well do it correctly. (For chromium's unit_tests target, 134421/242639 stub calls are direct calls without this change, compared to 134408/242639 with this change)
No real, meaningful behavior difference.
https://reviews.llvm.org/D108924
Files:
lld/MachO/ConcatOutputSection.cpp
Index: lld/MachO/ConcatOutputSection.cpp
===================================================================
--- lld/MachO/ConcatOutputSection.cpp
+++ lld/MachO/ConcatOutputSection.cpp
@@ -141,7 +141,7 @@
ThunkInfo &thunkInfo = thunkMap[sym];
// Knowing ThunkInfo call site count will help us know whether or not we
// might need to create more for this referent at the time we are
- // estimating distance to __stubs in .
+ // estimating distance to __stubs in estimateStubsInRangeVA().
++thunkInfo.callSiteCount;
// Knowing InputSection call site count will help us avoid work on those
// that have no BRANCH relocs.
@@ -153,28 +153,34 @@
// Since __stubs is placed after __text, we must estimate the address
// beyond which stubs are within range of a simple forward branch.
+// This is called exactly once, when the last input section has been finalized.
uint64_t ConcatOutputSection::estimateStubsInRangeVA(size_t callIdx) const {
- size_t endIdx = inputs.size();
- ConcatInputSection *isec = inputs[callIdx];
- uint64_t isecVA = isec->getVA();
- // Tally the non-stub functions which still have call sites
- // remaining to process, which yields the maximum number
- // of thunks we might yet place.
+ // Tally the functions which still have call sites remaining to process,
+ // which yields the maximum number of thunks we might yet place.
size_t maxPotentialThunks = 0;
for (auto &tp : thunkMap) {
ThunkInfo &ti = tp.second;
- maxPotentialThunks +=
- !tp.first->isInStubs() && ti.callSitesUsed < ti.callSiteCount;
+ // This overcounts: Only sections that are in forward jump range from the
+ // currently-active section get finalized, and all input sections are
+ // finalized when estimateStubsInRangeVA() is called. So only backward
+ // jumps will need thunks, but we count them all.
+ if (ti.callSitesUsed < ti.callSiteCount)
+ maxPotentialThunks += 1;
}
// Tally the total size of input sections remaining to process.
- uint64_t isecEnd = isec->getVA();
- for (size_t i = callIdx; i < endIdx; i++) {
+ uint64_t isecVA = inputs[callIdx]->getVA();
+ uint64_t isecEnd = isecVA;
+ for (size_t i = callIdx; i < inputs.size(); i++) {
InputSection *isec = inputs[i];
isecEnd = alignTo(isecEnd, isec->align) + isec->getSize();
}
// Estimate the address after which call sites can safely call stubs
// directly rather than through intermediary thunks.
uint64_t forwardBranchRange = target->forwardBranchRange;
+ assert(isecEnd > forwardBranchRange &&
+ "should not run thunk insertion if all code fits in jump range");
+ assert(isecEnd - isecVA <= forwardBranchRange &&
+ "should only finalize sections in jump range");
uint64_t stubsInRangeVA = isecEnd + maxPotentialThunks * target->thunkSize +
in.stubs->getSize() - forwardBranchRange;
log("thunks = " + std::to_string(thunkMap.size()) +
@@ -264,6 +270,7 @@
ThunkInfo &thunkInfo = thunkMap[funcSym];
// The referent is not reachable, so we need to use a thunk ...
if (funcSym->isInStubs() && callVA >= stubsInRangeVA) {
+ assert(callVA != TargetInfo::outOfRangeVA);
// ... Oh, wait! We are close enough to the end that __stubs
// are now within range of a simple forward branch.
continue;
@@ -284,7 +291,7 @@
continue;
}
}
- // ... otherwise, create a new thunk
+ // ... otherwise, create a new thunk.
if (isecAddr > highVA) {
// When there is small-to-no margin between highVA and
// isecAddr and the distance between subsequent call sites is
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108924.369473.patch
Type: text/x-patch
Size: 3711 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210830/5368cfbc/attachment.bin>
More information about the llvm-commits
mailing list