Skip to content

[lld-macho] Improve ICF thunk folding logic #131186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

alx32
Copy link
Contributor

@alx32 alx32 commented Mar 13, 2025

Refactor to add some early return logic to applySafeThunksToRange so that we completely skip irrelevant ranges.

Also add a check for isCodeSection to ensure we only apply branch thunks to code section (they don't make sense for anything else). Currently this isn't an issue since there are no keepUnique non-code sections - but this is not a hard restriction and may be implemented in the future, so we should be able to handle (i.e. avoid) this scenario.

@alx32 alx32 requested review from int3 and kyulee-com March 13, 2025 18:26
@alx32 alx32 marked this pull request as ready for review March 13, 2025 18:43
@alx32 alx32 requested a review from thevinster March 13, 2025 18:43
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2025

@llvm/pr-subscribers-lld-macho

@llvm/pr-subscribers-lld

Author: None (alx32)

Changes

Refactor to add some early return logic to applySafeThunksToRange so that we completely skip irrelevant ranges.

Also add a check for isCodeSection to ensure we only apply branch thunks to code section (they don't make sense for anything else). Currently this isn't an issue since there are no keepUnique non-code sections - but this is not a hard restriction and may be implemented in the future, so we should be able to handle (i.e. avoid) this scenario.


Full diff: https://github.com/llvm/llvm-project/pull/131186.diff

1 Files Affected:

  • (modified) lld/MachO/ICF.cpp (+14-5)
diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp
index acba0a2a824b4..9b8aa1837a4aa 100644
--- a/lld/MachO/ICF.cpp
+++ b/lld/MachO/ICF.cpp
@@ -297,16 +297,25 @@ static Symbol *getThunkTargetSymbol(ConcatInputSection *isec) {
 // direct branch thunk rather than containing a full copy of the actual function
 // body.
 void ICF::applySafeThunksToRange(size_t begin, size_t end) {
+  // When creating a unique ICF thunk, use the first section as the section that
+  // all thunks will branch to.
+  ConcatInputSection *masterIsec = icfInputs[begin];
+
+  // If the first section is not address significant, sorting guarantees that
+  // there are no address significant functions. So we can skip this range.
+  if (!masterIsec->keepUnique)
+    return;
+
+  // Skip anything that is not a code section.
+  if (!isCodeSection(masterIsec))
+    return;
+
   // If the functions we're dealing with are smaller than the thunk size, then
   // just leave them all as-is - creating thunks would be a net loss.
   uint32_t thunkSize = target->getICFSafeThunkSize();
-  if (icfInputs[begin]->data.size() <= thunkSize)
+  if (masterIsec->data.size() <= thunkSize)
     return;
 
-  // When creating a unique ICF thunk, use the first section as the section that
-  // all thunks will branch to.
-  ConcatInputSection *masterIsec = icfInputs[begin];
-
   // Get the symbol that all thunks will branch to.
   Symbol *masterSym = getThunkTargetSymbol(masterIsec);
 

@alx32 alx32 requested a review from DataCorrupted March 14, 2025 00:51
Copy link
Member

@DataCorrupted DataCorrupted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case all LGTM

@alx32 alx32 merged commit 8988914 into llvm:main Mar 14, 2025
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants