Skip to content

Conversation

@androm3da
Copy link
Contributor

Implements proper variadic argument handling for hexagon-unknown-linux-musl targets using a 3-pointer VaList structure compatible with LLVM's HexagonBuiltinVaList implementation.

  • Handles register save area vs overflow area transition
  • Provides proper 4-byte and 8-byte alignment for arguments
  • Only activates for hexagon+musl targets via Arch::Hexagon & Env::Musl

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot

This comment has been minimized.

@androm3da
Copy link
Contributor Author

cc @folkertdev

Tested with #150009 #150010, and this config.toml:

# Rust build configuration for hexagon-unknown-linux-musl

[build]
# Build the hexagon-unknown-linux-musl target
target = ["x86_64-unknown-linux-gnu", "hexagon-unknown-linux-musl"]

[rust]
# Build the standard library for hexagon-unknown-linux-musl
std = true

# Enable codegen tests to verify the target works
codegen-tests = true

# Use local libc patches
optimize = false

[dist]
# Include the hexagon-unknown-linux-musl target in distributions
missing-tools = true

[target.hexagon-unknown-linux-musl]
# Use hexagon-linux-musl-clang as the linker
cc = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/hexagon-linux-musl-clang"
cxx = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/hexagon-linux-musl-clang++"
linker = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/hexagon-linux-musl-clang"
ar = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/hexagon-linux-musl-ar"
ranlib = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/hexagon-linux-musl-ranlib"
musl-root = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/target/hexagon-unknown-linux-musl/usr"

# Configure qemu-hexagon as the test runner for this target
runner = "/opt/clang+llvm-20.1.4-cross-hexagon-unknown-linux-musl/x86_64-linux-gnu/bin/qemu-hexagon"

Implements proper variadic argument handling for hexagon-unknown-linux-musl
targets using a 3-pointer VaList structure compatible with LLVM's
HexagonBuiltinVaList implementation.

* Handles register save area vs overflow area transition
* Provides proper 4-byte and 8-byte alignment for arguments
* Only activates for hexagon+musl targets via Arch::Hexagon & Env::Musl
@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@lcnr
Copy link
Contributor

lcnr commented Dec 15, 2025

@rustbot reroll

@rustbot rustbot assigned SparrowLii and unassigned lcnr Dec 15, 2025
@folkertdev
Copy link
Contributor

r? folkertdev

@rustbot rustbot assigned folkertdev and unassigned SparrowLii Dec 15, 2025
Copy link
Contributor

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

On its own this will miscompile, so maybe you can copy the changes from #149967 and just add them to this commit.

View changes since this review

@androm3da
Copy link
Contributor Author

On its own this will miscompile, so maybe you can copy the changes from #149967 and just add them to this commit.

View changes since this review

Oh - oops. Or maybe I you can cherry-pick it on top of #149967? No need to make a separate PR if it should just go in that one, I suppose.

@androm3da androm3da changed the title Implement va_arg for Hexagon Linux musl targets Implement va_arg for Hexagon targets Dec 16, 2025
Copy link
Contributor

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

I'll just merge this when the other PR is merged then

View changes since this review


// Calculate offset: round up type size to 4-byte boundary (minimum stack slot size)
let type_size = layout.size.bytes();
let offset = ((type_size + 3) / 4) * 4; // align to 4 bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that is

Suggested change
let offset = ((type_size + 3) / 4) * 4; // align to 4 bytes
let offset = type_size.next_multiple_of(4); // align to 4 bytes

Comment on lines +868 to +872
fn emit_hexagon_va_arg_bare_metal<'ll, 'tcx>(
bx: &mut Builder<'_, 'll, 'tcx>,
list: OperandRef<'tcx, &'ll Value>,
target_ty: Ty<'tcx>,
) -> &'ll Value {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this different from emit_ptr_va_arg?

Copy link
Contributor

Choose a reason for hiding this comment

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

Like, I know that this is what LLVM does, but unless I'm missing something LLVM implementation could also use emitVoidPtrVAArg

// This includes `target.is_like_darwin`, which on x86_64 targets is like sysv64.
Arch::X86_64 => emit_x86_64_sysv64_va_arg(bx, addr, target_ty),
Arch::Xtensa => emit_xtensa_va_arg(bx, addr, target_ty),
Arch::Hexagon => emit_hexagon_va_arg(bx, addr, target_ty, target.env == Env::Musl),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just inline emit_hexagon_va_arg here

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

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants