Skip to content

Enable prioritized linker optimization for AArch64 in setup.py and clean up CI script #160078

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

Open
wants to merge 1 commit into
base: gh/robert-hardwick/2/base
Choose a base branch
from

Conversation

robert-hardwick
Copy link
Collaborator

@robert-hardwick robert-hardwick commented Aug 7, 2025

Note. This is a replica PR of #155901 which will be closed. I had to create a new PR in order to add it into my ghstack as there are some later commits which depend on it.

Summary

🚀 This PR enables prioritized text linker optimization by default on Linux aarch64 systems via logic added to setup.py.

If USE_PRIORITIZED_TEXT_FOR_LD is not already set, the build system will now automatically:
• Set it to 1 on Linux + aarch64
• Inject appropriate linker and compiler flags via LDFLAGS, CFLAGS, and CXXFLAGS

This change consolidates what was previously manual CI logic into a single location (setup.py), ensuring consistent behavior across local builds, CI pipelines, and developer environments.

Motivation

Prioritized text layout has measurable performance benefits on Arm systems by reducing code padding and improving cache utilization. This optimization was previously triggered manually via CI scripts (.ci/aarch64_linux/aarch64_ci_build.sh) or user-set environment variables. By detecting the target architecture within setup.py, this change enables the optimization automatically where applicable, improving maintainability and usability.

Stack from ghstack (oldest at bottom):

Co-authored-by: Usamah Zaheer usamah.zaheer@arm.com

cc @malfet @snadampal @milpuz01 @aditew01 @nikhil-arm @fadara01 @usamahz

[ghstack-poisoned]
@robert-hardwick robert-hardwick requested a review from a team as a code owner August 7, 2025 10:53
Copy link

pytorch-bot bot commented Aug 7, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/160078

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (3 Unrelated Failures)

As of commit 62e5e8c with merge base 9a680e1 (image):

UNSTABLE - The following jobs are marked as unstable, possibly due to flakiness on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@robert-hardwick
Copy link
Collaborator Author

Going to close #155901 for this PR. It needed to be rebased and #160079 depends on it, so thought it would be easier to bring into the ghstack.

@robert-hardwick
Copy link
Collaborator Author

@pytorchbot label "ciflow/linux-aarch64"

@pytorch-bot pytorch-bot bot added the ciflow/linux-aarch64 linux aarch64 CI workflow label Aug 7, 2025
@robert-hardwick robert-hardwick added module: arm Related to ARM architectures builds of PyTorch. Includes Apple M1 and removed ciflow/linux-aarch64 linux aarch64 CI workflow labels Aug 7, 2025
@robert-hardwick robert-hardwick requested a review from tinglvv August 7, 2025 11:10
@robert-hardwick
Copy link
Collaborator Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Aug 7, 2025
@robert-hardwick
Copy link
Collaborator Author

@pytorchbot label "ciflow/linux-aarch64"

@pytorch-bot pytorch-bot bot added the ciflow/linux-aarch64 linux aarch64 CI workflow label Aug 7, 2025
@robert-hardwick robert-hardwick changed the title Enable prioritized linker optimization for AArch64 in setup.py and clean up CI script - cherry-pick from 155901 Enable prioritized linker optimization for AArch64 in setup.py and clean up CI script Aug 7, 2025
@@ -1243,12 +1243,27 @@ def main() -> None:
if BUILD_PYTHON_ONLY:
install_requires += [f"{LIBTORCH_PKG_NAME}=={TORCH_VERSION}"]

if (
os.getenv("USE_PRIORITIZED_TEXT_FOR_LD") is None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this logic live in the CMake?

Copy link
Collaborator Author

@robert-hardwick robert-hardwick Aug 8, 2025

Choose a reason for hiding this comment

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

Hmm so we want to get CMake to call this python function
https://github.com/pytorch/pytorch/blob/main/tools/setup_helpers/generate_linker_script.py before the build if a certain platform and OS is set.

https://cmake.org/cmake/help/latest/command/add_custom_command.html#build-events

I'll investigate if it can be done.

@@ -1258,7 +1273,7 @@ def main() -> None:
elif platform.system() == "Linux" and platform.processor() == "aarch64":
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just looking at this in more detail, this condition will never be True because of line 1251, so we can delete 1273-1279.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/linux-aarch64 linux aarch64 CI workflow module: arm Related to ARM architectures builds of PyTorch. Includes Apple M1 open source topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants