Skip to content

EXPERIMENT: build: Add musl linker flags to reduce OpenHCL size - #4317

Open
Steven Malis (smalis-msft) wants to merge 2 commits into
microsoft:mainfrom
smalis-msft:linker-opt
Open

EXPERIMENT: build: Add musl linker flags to reduce OpenHCL size#4317
Steven Malis (smalis-msft) wants to merge 2 commits into
microsoft:mainfrom
smalis-msft:linker-opt

Conversation

@smalis-msft

Copy link
Copy Markdown
Contributor

Use rustup's provided lld and turn on some linker optimizations to shrink OpenHCL. Note that this does come with some debuggability downsides.

Copilot AI lite review requested due to automatic review settings August 26, 2026 14:22
@smalis-msft
Steven Malis (smalis-msft) requested a review from a team as a code owner August 26, 2026 14:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR experiments with reducing OpenHCL binary size for musl targets by switching to the Rust toolchain’s bundled lld and enabling linker optimizations (notably ICF and lld -O2), accepting some debuggability tradeoffs.

Changes:

  • Add musl-target rustflags to link with lld and enable --icf=all + lld -O2 in .cargo/config.toml.
  • Update the Underhill musl gcc wrapper scripts to direct GCC to the Rust toolchain’s gcc-ld directory so -fuse-ld=lld works without a distro lld install.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
build_support/underhill_cross/x86_64-underhill-musl-gcc Points GCC at the Rust toolchain’s gcc-ld location so lld can be found when -fuse-ld=lld is used.
build_support/underhill_cross/aarch64-underhill-musl-gcc Same as above for aarch64 musl.
.cargo/config.toml Enables lld + size-reduction flags for all musl builds via Cargo rustflags.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +17 to +22
# Satisfy the -fuse-ld=lld in .cargo/config.toml using the lld bundled with the
# Rust toolchain, so no distro lld package is needed. gcc-ld only ever exists
# for the host triple; if it is missing, gcc falls back to searching PATH.
gcc_ld_dir="$(rustc --print sysroot)/lib/rustlib/$(rustc -vV | sed -n 's/^host: //p')/bin/gcc-ld"

MUSL_ARCH=x86_64 MUSL_SYSROOT="$X86_64_SYSROOT" exec "$REALGCC" "$@" -B"$gcc_ld_dir" -specs "$script_dir/musl-gcc.specs"
Comment on lines +17 to +22
# Satisfy the -fuse-ld=lld in .cargo/config.toml using the lld bundled with the
# Rust toolchain, so no distro lld package is needed. gcc-ld only ever exists
# for the host triple; if it is missing, gcc falls back to searching PATH.
gcc_ld_dir="$(rustc --print sysroot)/lib/rustlib/$(rustc -vV | sed -n 's/^host: //p')/bin/gcc-ld"

MUSL_ARCH=aarch64 MUSL_SYSROOT="$AARCH64_SYSROOT" exec "$REALGCC" "$@" -B"$gcc_ld_dir" -specs "$script_dir/musl-gcc.specs"
@github-actions

Copy link
Copy Markdown

Copilot AI review requested due to automatic review settings September 3, 2026 23:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The new musl GCC wrapper logic depends on rustc output but currently lacks validation/error handling, which can cause confusing build failures when rustc/toolchain paths are unavailable or unparsable.

Review details

Suppressed comments (2)

build_support/underhill_cross/x86_64-underhill-musl-gcc:22

  • gcc_ld_dir is derived from rustc output without validating that rustc is available or that the computed gcc-ld directory exists. If rustc is missing/misconfigured (or sed returns an empty host triple), the wrapper will pass -B with a missing argument or an invalid path, producing a confusing GCC error instead of a clear failure mode.
gcc_ld_dir="$(rustc --print sysroot)/lib/rustlib/$(rustc -vV | sed -n 's/^host: //p')/bin/gcc-ld"

MUSL_ARCH=x86_64 MUSL_SYSROOT="$X86_64_SYSROOT" exec "$REALGCC" "$@" -B"$gcc_ld_dir" -specs "$script_dir/musl-gcc.specs"

build_support/underhill_cross/aarch64-underhill-musl-gcc:22

  • gcc_ld_dir is computed from rustc output without checking for rustc availability or verifying that the resulting gcc-ld directory exists. If rustc isn't on PATH (or the host triple can't be parsed), this wrapper can end up invoking GCC with an invalid -B argument and fail with a non-obvious error.
gcc_ld_dir="$(rustc --print sysroot)/lib/rustlib/$(rustc -vV | sed -n 's/^host: //p')/bin/gcc-ld"

MUSL_ARCH=aarch64 MUSL_SYSROOT="$AARCH64_SYSROOT" exec "$REALGCC" "$@" -B"$gcc_ld_dir" -specs "$script_dir/musl-gcc.specs"
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Restore packages

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants