Skip to content

LLBC-linker and NVPTX: do not strip debug when PTX ISA version >= 7.0#147672

Open
kjetilkjeka wants to merge 1 commit intorust-lang:mainfrom
kjetilkjeka:llbc-linker-nvptx-strip-less-debug
Open

LLBC-linker and NVPTX: do not strip debug when PTX ISA version >= 7.0#147672
kjetilkjeka wants to merge 1 commit intorust-lang:mainfrom
kjetilkjeka:llbc-linker-nvptx-strip-less-debug

Conversation

@kjetilkjeka
Copy link
Copy Markdown
Contributor

@kjetilkjeka kjetilkjeka commented Oct 14, 2025 •
edited by rustbot
Loading

Related to #99248

With the version bump of llvm happening a while back it's possible to not strip debug symbols in llbc-linker for the nvptx target in a lot of useful cases.

Instead of always stripping for nvptx64 the new behavior is to look at target-feature and target-cpu and strip for combinations that produce old enough ptx isa version to trigger the bug.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 14, 2025
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Oct 14, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

@kjetilkjeka
Copy link
Copy Markdown
Contributor Author

kjetilkjeka commented Oct 14, 2025

@rustbot label +O-NVPTX

@rustbot rustbot added the O-NVPTX Target: the NVPTX LLVM backend for running rust on GPUs, https://llvm.org/docs/NVPTXUsage.html label Oct 14, 2025
@Mark-Simulacrum
Copy link
Copy Markdown
Member

Mark-Simulacrum commented Nov 2, 2025

@RDambrosio016, can you take a look as the other target maintainer for nvptx?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 7, 2025
@kjetilkjeka
Copy link
Copy Markdown
Contributor Author

kjetilkjeka commented Dec 19, 2025 •
edited
Loading

@Mark-Simulacrum I see that you swapped the waiting for labels. If this is related to waiting for @RDambrosio016 I don't expect him to check in here. It seems like he's not been checking notifications related to rust-lang for a while.

Is there anything I can do to make progress on this PR?

@kjetilkjeka kjetilkjeka mentioned this pull request Feb 1, 2026
Closed
3 tasks
@kjetilkjeka
LLBC-linker: do not unconditionally strip debug symbols for nvptx
874fb5f 
This was an issue for old ptx ISA versions. Those are not supported by rustc anymore
@kjetilkjeka kjetilkjeka force-pushed the llbc-linker-nvptx-strip-less-debug branch from ddbcae9 to 874fb5f Compare May 2, 2026 20:16
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 2, 2026

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.

@kjetilkjeka
Copy link
Copy Markdown
Contributor Author

kjetilkjeka commented May 2, 2026 •
edited
Loading

We do not support ptx isa older than 7.0 anymore and AFAIK LLVM will produce valid debug symbols for that version and more recent. That means no special case is needed to remove the stripping of debug symbol hack and that should make this PR a pure improvement.

Maybe you would consider adopting the review part of it @ZuseZ4 ? Or maybe this is outside the domain of the compiler team?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 3, 2026
ZuseZ4
ZuseZ4 reviewed May 4, 2026
View reviewed changes
Comment thread src/tools/llvm-bitcode-linker/src/linker.rs
let mut passes = format!("default<{}>", optimization);

// FIXME(@kjetilkjeka) Debug symbol generation is broken for nvptx64 so we must remove them even in debug mode
if debug && self.target == crate::Target::Nvptx64NvidiaCuda {
Copy link
Copy Markdown
Member

@ZuseZ4 ZuseZ4 May 4, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Can you add a test to check for some of those debug symbols?

View changes since the review

@ZuseZ4
Copy link
Copy Markdown
Member

ZuseZ4 commented May 4, 2026

Nice range-dif. Edition 2024 seems fine, other tools also require it.
Generally lgtm, now that the PR has become trivial due to dropping old versions. Still, can you turn one of the examples from the linked issue into a test?
Mark is quite busy iirc, so let me ask if someone else from infra has time to approve or would be ok if I approve it later this week.

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

Reviewers

@ZuseZ4 ZuseZ4 ZuseZ4 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels

O-NVPTX Target: the NVPTX LLVM backend for running rust on GPUs, https://llvm.org/docs/NVPTXUsage.html S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kjetilkjeka @rustbot @Mark-Simulacrum @ZuseZ4