Skip to content

Updating target json support #256

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

Firestar99
Copy link
Member

@Firestar99 Firestar99 commented May 21, 2025

cargo gpu: Rust-GPU/cargo-gpu#75

This is the first PR to update the target spec jsons, which causes cargo gpu to run into some trouble. The codegen backend verifies that these jsons match exactly what it expects, and there's no flag to skip that check, and adding one now wouldn't help since we want to keep compatible with at least 0.9.0. Cargo gpu was never designed to have multiple target-spec versions, it always just used the latest. [...]

#249 (comment)

Could we move the target jsons from spirv_builder to rustc_codegen_spirv-types? That would allow cargo-gpu to resolve the required target jsons from rustc_codegen_spirv depending on rustc_codegen_spirv-types (and spirv_builder will retain access to it). If we can't find any target jsons in rustc_codegen_spirv-types, we can just fall back on the current target jsons, which we'd have to ship with cargo-gpu for the forseeable future. But I think that's a fine compromise, at least we wouldn't need to ship all future target jsons as well.

#249 (comment)

@Firestar99 Firestar99 marked this pull request as ready for review May 21, 2025 17:26
@LegNeato
Copy link
Collaborator

Should this be its own crate? @eddyb and I discussed it in the past.

@Firestar99
Copy link
Member Author

You mean just the target specs json files as a separate crate? And have cargo gpu to just permanently depend on the "legacy" version of the target specs? (legacy meaning before this PR)

Just one issue with that: spirv-builder wants the path to the json files so it can point rustc to them, which is ensured to exist by directly depending on that crate. But in cargo gpu the legacy target jsons are include_str! into the binary, so we don't depend on those legacy json files remaining within cargo's crate cache. Which may be reset independently of the cargo gpu installation.

@LegNeato
Copy link
Collaborator

LegNeato commented May 21, 2025

Yeah the json as their own crate. Doing so lets us have multiple versions and doesn't bring in other deps if something just needs the json. As I said we discussed it in the past but I think just kicked the can down the road until problems cropped up. We may now be there ha

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.

2 participants