Conversation
Firestar99
force-pushed
the
update-toolchain-2025-11-13
branch
from
8611b98 to
7114256
Compare
Firestar99
force-pushed
the
update-toolchain-2025-11-13
branch
from
7114256 to
c79e9f1
Compare
Firestar99
force-pushed
the
update-toolchain-2025-11-13
branch
3 times, most recently
from
6c87ef0 to
237121f
Compare
|
BTW you can change the target branch to that of #543 so that it shows a smaller more focused diff. The target branch will change to |
| // Note(@firestar99): not sure if this does anything | ||
| o.is_like_gpu = true; |
There was a problem hiding this comment.
I remember a recent change in the compiler being one that prevents recursive static definitions which could be tied to this
fee1-dead
force-pushed
the
update-toolchain-2025-11-02
branch
from
05a7b6a to
735c718
Compare
LegNeato
force-pushed
the
update-toolchain-2025-11-13
branch
from
370ca91 to
6714ee3
Compare
- Rewrite extern C fn sigs to Rust ABI to satisfy new rustc fn_abi sanity checks. - Handle direct calls represented as ptr<void> by recovering function signature from callee type/fn_abi. - Keep generated SPIR-V target JSON aligned with backend target options and rustc 1.93 schema expectations.
Refresh expected stderr for debug_printf trait-path wording and unwrap_or line mapping changes.
Normalize target-spec JSON before comparing backend-expected vs provided JSON by dropping the is-like-gpu key, which rustc rejects in external target JSON for non-nvptx/amdgcn targets.
LegNeato
force-pushed
the
update-toolchain-2025-11-13
branch
from
6714ee3 to
3bbb1fd
Compare
|
@LegNeato I think I want to take some time to review the ABI changes, given bjorn3's suggestion in the other thread, I am not sure if this is the approach to take |
| // NOTE: this used to rewrite to `extern "unadjusted"`, but rustc now | ||
| // validates `#[rustc_pass_indirectly_in_non_rustic_abis]` for non-Rust ABIs, | ||
| // and `Unadjusted` does not satisfy that requirement. | ||
| providers.fn_sig = |tcx, def_id| { | ||
| // We can't capture the old fn_sig and just call that, because fn_sig is a `fn`, not a `Fn`, i.e. it can't | ||
| // capture variables. Fortunately, the defaults are exposed (thanks rustdoc), so use that instead. | ||
| let result = (rustc_interface::DEFAULT_QUERY_PROVIDERS.fn_sig)(tcx, def_id); | ||
| result.map_bound(|outer| { | ||
| outer.map_bound(|mut inner| { | ||
| if let Abi::C { .. } = inner.abi { | ||
| inner.abi = Abi::Unadjusted; | ||
| inner.abi = Abi::Rust; |
There was a problem hiding this comment.
I think this the ultimately the right direction. But I'm really curious as to whether this is actually enough of a change for this to be correct. bjorn3's comment on the other PR: #460 (comment) says that assuming that we can't deal with PassMode::Indirect, this could technically still make us use PassMode::Indirect.
Maybe the assertion got passed since it only checks it when the ABI is not rustic, so Abi::Rust passes it (cc https://github.com/rust-lang/rust/blob/0b329f801a09004dacb19aaf09d5cb8b4c51d3f8/compiler/rustc_ty_utils/src/abi.rs#L392-L397), but I do think we might want to look into whether or not we have caused more functions to use PassMode::Indirect and whether we have enough support for that.
There was a problem hiding this comment.
So given this, I think this is good enough to go for now, but I'd want to do some testing as to whether something broke. Did the compiletests cover things that should have been covered or are we introducing a regression? I suppose we don't know much but I'm not opposed to just landing this and doing more followup work on it.
5 participants
Requires #541, conflicts with #491
A full rebase of #460, still ICEing a lot. See that PR for ideas.