Nvptx remove direct passmode#125980
Merged
bors merged 2 commits intorust-lang:masterfrom Jun 12, 2024
Merged
Conversation
added 2 commits
May 31, 2024 22:45
…ince it was used as an example when discussing possible problems
Collaborator
|
rustbot has assigned @petrochenkov. Use |
rustbot
added
S-waiting-on-review
Contributor
Author
|
Seems like rustbot was not happy with @RalfJung as reviewer. I will request @davidtwco due to knowing this topic from #117671 r? @davidtwco |
This was referenced Jun 4, 2024
Contributor
Author
|
@rustbot label +O-NVPTX |
rustbot
added
the
O-NVPTX
Member
|
Not sure why it ignored the |
davidtwco
approved these changes
Jun 12, 2024
Member
|
@bors r+ rollup |
Collaborator
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 12, 2024
…kingjubilee Rollup of 16 pull requests Successful merges: - rust-lang#123374 (DOC: Add FFI example for slice::from_raw_parts()) - rust-lang#124514 (Recommend to never display zero disambiguators when demangling v0 symbols) - rust-lang#125978 (Cleanup: HIR ty lowering: Consolidate the places that do assoc item probing & access checking) - rust-lang#125980 (Nvptx remove direct passmode) - rust-lang#126187 (For E0277 suggest adding `Result` return type for function when using QuestionMark `?` in the body.) - rust-lang#126210 (docs(core): make more const_ptr doctests assert instead of printing) - rust-lang#126249 (Simplify `[T; N]::try_map` signature) - rust-lang#126256 (Add {{target}} substitution to compiletest) - rust-lang#126263 (Make issue-122805.rs big endian compatible) - rust-lang#126281 (set_env: State the conclusion upfront) - rust-lang#126286 (Make `storage-live.rs` robust against rustc internal changes.) - rust-lang#126287 (Update a cranelift patch file for formatting changes.) - rust-lang#126301 (Use `tidy` to sort crate attributes for all compiler crates.) - rust-lang#126305 (Make PathBuf less Ok with adding UTF-16 then `into_string`) - rust-lang#126310 (Migrate run make prefer rlib) - rust-lang#126314 (fix RELEASES: we do not support upcasting to auto traits) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 12, 2024
Rollup merge of rust-lang#125980 - kjetilkjeka:nvptx_remove_direct_passmode, r=davidtwco Nvptx remove direct passmode This PR does what should have been done in rust-lang#117671. That is fully avoid using the `PassMode::Direct` for `extern "C" fn` for `nvptx64-nvidia-cuda` and enable the compatibility test. `@RalfJung` [pointed me in the right direction](rust-lang#117480 (comment)) for solving this issue. There are still some ABI bugs after this PR is merged. These ABI tests are created based on what is actually correct, and since they continue passing with even more of them enabled things are improving. I don't have the time to tackle all the remaining issues right now, but I think getting these improvements merged is very valuable in themselves and plan to tackle more of them long term. This also doesn't remove the use of `PassMode::Direct` for `extern "ptx-kernel" fn`. This was also not trivial to make work. And since the ABI is hidden behind an unstable feature it's less urgent. I don't know if it's correct to request `@RalfJung` as a reviewer (due to team structures), but he helped me a lot to figure out this stuff. If that's not appropriate then `@davidtwco` would be a good candidate since he know about this topic from rust-lang#117671 r? `@RalfJung`
bjorn3
pushed a commit
to bjorn3/rust
that referenced
this pull request
Jun 30, 2024
…kingjubilee Rollup of 16 pull requests Successful merges: - rust-lang#123374 (DOC: Add FFI example for slice::from_raw_parts()) - rust-lang#124514 (Recommend to never display zero disambiguators when demangling v0 symbols) - rust-lang#125978 (Cleanup: HIR ty lowering: Consolidate the places that do assoc item probing & access checking) - rust-lang#125980 (Nvptx remove direct passmode) - rust-lang#126187 (For E0277 suggest adding `Result` return type for function when using QuestionMark `?` in the body.) - rust-lang#126210 (docs(core): make more const_ptr doctests assert instead of printing) - rust-lang#126249 (Simplify `[T; N]::try_map` signature) - rust-lang#126256 (Add {{target}} substitution to compiletest) - rust-lang#126263 (Make issue-122805.rs big endian compatible) - rust-lang#126281 (set_env: State the conclusion upfront) - rust-lang#126286 (Make `storage-live.rs` robust against rustc internal changes.) - rust-lang#126287 (Update a cranelift patch file for formatting changes.) - rust-lang#126301 (Use `tidy` to sort crate attributes for all compiler crates.) - rust-lang#126305 (Make PathBuf less Ok with adding UTF-16 then `into_string`) - rust-lang#126310 (Migrate run make prefer rlib) - rust-lang#126314 (fix RELEASES: we do not support upcasting to auto traits) r? `@ghost` `@rustbot` modify labels: rollup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR does what should have been done in #117671. That is fully avoid using the
PassMode::Directforextern "C" fnfornvptx64-nvidia-cudaand enable the compatibility test. @RalfJung pointed me in the right direction for solving this issue.There are still some ABI bugs after this PR is merged. These ABI tests are created based on what is actually correct, and since they continue passing with even more of them enabled things are improving. I don't have the time to tackle all the remaining issues right now, but I think getting these improvements merged is very valuable in themselves and plan to tackle more of them long term.
This also doesn't remove the use of
PassMode::Directforextern "ptx-kernel" fn. This was also not trivial to make work. And since the ABI is hidden behind an unstable feature it's less urgent.I don't know if it's correct to request @RalfJung as a reviewer (due to team structures), but he helped me a lot to figure out this stuff. If that's not appropriate then @davidtwco would be a good candidate since he know about this topic from #117671
r? @RalfJung