Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hack around blockers to update toolchain all the way to nightly-2024-11-22 (~1.84). #170

Merged
merged 11 commits into from
Dec 18, 2024

Conversation

eddyb
Copy link
Collaborator

@eddyb eddyb commented Dec 3, 2024

Each commit should be reviewed separately.

(in theory, I still need to test it, which is why this is "Draft" - ideally we can test debug mode builds in CI? hopefully without a lot of time wasted etc. - also, are there any more issues that updating nightly fixes?)

That issue is in fact how I got the idea for the worst hack of this (see the very end of #29 (comment)).


The terrible hacks making this possible:

  • rustc_codegen_ssa's source code is now copied out of the sysroot and patched
    • (see crates/rustc_codegen_spirv/build.rs for most of the implementation)
    • allows reintroducing typed (instead of merely (Size, Align)-shaped)
      allocas (LLVM's name for "stack slots"/function-local variables),
      previously removed in Stop using LLVM struct types for alloca rust-lang/rust#122053
    • the modified version is dubbed pqp_cg_ssa to make it harder to confuse
      (where "pqp" = "pre-qptr-patched", a nod to the eventual real solution)
    • most of the difficulty isn't with getting the rustc_codegen_ssa source
      (as the rustc-dev rustup component places it in the sysroot),
      or the patches (most of which are very simple!), but rather the build
      ("nesting a crate as a module in another crate" + Cargo deps issues)
    • unblocks updating beyond nightly-2024-04-24 (last nightly before that PR)
  • check_well_formed query is bypassed for very simple #[repr(simd)] structs
    • effectively undoes (for now) Ban non-array SIMD rust-lang/rust#129403
    • avoids having to come up with a replacement for our use of #[repr(simd)],
      and moving glam over to it
      (something as simple as #[spirv(vector)] might work, but 1. we didn't want glam depending on spirv-std* crates and 2. I haven't tried this theory, we might be relying a lot on rustc's SIMD concepts)
    • unblocks updating to nightly-2024-10-12 (~1.83) and beyond
  • check_mono_item query is completely disabled (replaced with a noop)

We're lucky that 1.83 just released, so the "~1.84" nightly this PR gets to is currently in beta (to become stable 1.84 in 5 more weeks or so), so if we release Rust-GPU 0.10 in the next few weeks, we won't be behind stable at all.


There is also at least one refactor (SpirvConst::Scalar) which could be moved into its own PR, as well as a much smaller fix for panic_nounwind_fmt (not directly relevant, just that the format_args! "decompiler" changes I had for some of these nightlies had assumed that change originally).

@eddyb eddyb force-pushed the rustup branch 2 times, most recently from 28929ce to 5642ab5 Compare December 3, 2024 19:21
Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

I'm not really qualified to review this, some superficial comments.

/// produce a "pqp" ("pre-`qptr`-patched") version that maintains compatibility
/// with "legacy" Rust-GPU pointer handling (mainly typed `alloca`s).
//
// FIXME(eddyb) get rid of this as soon as it's not needed anymore.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should have a task written for this so we don't lose track

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

crates/rustc_codegen_spirv/build.rs Show resolved Hide resolved
// or *at most* one generic type parameter with no bounds/where clause
// - relies on upstream `layout_of` not having had the non-array logic removed
//
// FIXME(eddyb) remove this once migrating beyond `#[repr(simd)]` becomes
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prob want a task for this too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

})
.unwrap_or(InsertPoint::End)
};
// TODO: rspirv doesn't have insert_variable function
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need a task on rspirv? If so we should file and link here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This stuff is an old comment that can probably removed (expecting high-level APIs from rspirv is counterproductive).

@eddyb
Copy link
Collaborator Author

eddyb commented Dec 10, 2024

Looking like codegen-units = 1 + -Zshare-generics=off was enough to push Windows under the MSVC (or possibly file format?) 64Ki limit.

Still have some comments (#170 (review)) to address, but otherwise this seems to work.

@LegNeato
Copy link
Collaborator

Are we ready to take this out of draft @eddyb ?

@eddyb eddyb marked this pull request as ready for review December 17, 2024 20:55
@eddyb eddyb requested a review from fornwall as a code owner December 17, 2024 20:55
@eddyb eddyb enabled auto-merge December 17, 2024 22:51
Copy link
Member

@Firestar99 Firestar99 left a comment

Choose a reason for hiding this comment

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

stamping as requested by @eddyb

@eddyb eddyb added this pull request to the merge queue Dec 18, 2024
Merged via the queue into Rust-GPU:main with commit 1d76d59 Dec 18, 2024
7 checks passed
@eddyb eddyb deleted the rustup branch December 18, 2024 11:35
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.

Segfault while compiling rustc_codegen_spirv
3 participants