Implement va_arg for Hexagon targets#150008

androm3da · 2025-12-15T01:24:14Z

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 · 2025-12-15T01:24:19Z

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

androm3da · 2025-12-15T01:28:13Z

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 · 2025-12-15T01:55:00Z

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 · 2025-12-15T09:17:49Z

@rustbot reroll

folkertdev · 2025-12-15T09:27:23Z

r? folkertdev

folkertdev

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

compiler/rustc_codegen_llvm/src/va_arg.rs

androm3da · 2025-12-15T13:38:13Z

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.

folkertdev

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

View changes since this review

folkertdev · 2025-12-16T10:38:35Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+
+    // 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


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

change taken, thanks

folkertdev · 2025-12-16T10:39:54Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+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 {


is this different from emit_ptr_va_arg?

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

I think you might be right that clang could use emitVoidPtrVAArg. Do you mind if I take that up separately with the hexagon compiler team and revisit it based on that discussion?

Sounds good.

folkertdev · 2025-12-16T10:40:38Z

compiler/rustc_codegen_llvm/src/va_arg.rs

        // 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),


I'd just inline emit_hexagon_va_arg here

rustbot · 2025-12-17T04:22:59Z

Some changes occurred in src/tools/cargo

cc @ehuss

folkertdev

Nice, thanks!

@bors r+

View changes since this review

folkertdev · 2025-12-17T09:51:48Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+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 {


Sounds good.

folkertdev · 2025-12-17T10:00:27Z

@bors r+ rollup

bors · 2025-12-17T10:00:30Z

📌 Commit 4ff4b25 has been approved by folkertdev

It is now in the queue for this repository.

…uwer Rollup of 3 pull requests Successful merges: - #150008 (Implement va_arg for Hexagon targets) - #150044 (Avoid unhelpful suggestion when crate name is invalid) - #150086 (Port `#[rustc_never_returns_null_ptr]` to attribute parser) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #150008 - androm3da:bcain/va_arg_rs, r=folkertdev Implement va_arg for Hexagon targets 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 assigned lcnr Dec 15, 2025

This comment has been minimized.

Sign in to view

androm3da force-pushed the bcain/va_arg_rs branch from 961715a to 0c1612e Compare December 15, 2025 01:54

androm3da mentioned this pull request Dec 15, 2025

custom VaList layout for Hexagon #149967

Merged

rustbot assigned SparrowLii and unassigned lcnr Dec 15, 2025

rustbot assigned folkertdev and unassigned SparrowLii Dec 15, 2025

folkertdev reviewed Dec 15, 2025

View reviewed changes

compiler/rustc_codegen_llvm/src/va_arg.rs Outdated Show resolved Hide resolved

fixup! Implement va_arg for Hexagon Linux musl targets

d8a0f80

androm3da changed the title ~~Implement va_arg for Hexagon Linux musl targets~~ Implement va_arg for Hexagon targets Dec 16, 2025

folkertdev reviewed Dec 16, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

androm3da force-pushed the bcain/va_arg_rs branch from fa4a020 to 0a956c5 Compare December 17, 2025 04:22

This comment has been minimized.

Sign in to view

fixup! Implement va_arg for Hexagon Linux musl targets

4ff4b25

androm3da force-pushed the bcain/va_arg_rs branch from 0a956c5 to 4ff4b25 Compare December 17, 2025 04:26

folkertdev approved these changes Dec 17, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 17, 2025

JonathanBrouwer mentioned this pull request Dec 17, 2025

Rollup of 3 pull requests #150089

Merged

bors merged commit 56c00b0 into rust-lang:main Dec 17, 2025
11 checks passed

rustbot added this to the 1.94.0 milestone Dec 17, 2025

androm3da mentioned this pull request Dec 17, 2025

draft: [hexagon] Switch to emitVoidPtrVAArg llvm/llvm-project#172704

Draft

github-actions bot mentioned this pull request Dec 22, 2025

Toolchain upgrade to nightly-2026-02-07 flux-rs/flux#1345

Open

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

Uh oh!

Conversation

androm3da commented Dec 15, 2025

Uh oh!

rustbot commented Dec 15, 2025

Uh oh!

This comment has been minimized.

androm3da commented Dec 15, 2025

Uh oh!

rustbot commented Dec 15, 2025

Uh oh!

lcnr commented Dec 15, 2025

Uh oh!

folkertdev commented Dec 15, 2025

Uh oh!

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

androm3da commented Dec 15, 2025

Uh oh!

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rustbot commented Dec 17, 2025

Uh oh!

This comment has been minimized.

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

folkertdev commented Dec 17, 2025

Uh oh!

bors commented Dec 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

folkertdev left a comment •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading