Skip to content

Fix codegen of Memory-repr enum payload accesses#616

Open
LegNeato wants to merge 1 commit into
Rust-GPU:mainfrom
LegNeato:issue615-usize-u32
Open

Fix codegen of Memory-repr enum payload accesses#616
LegNeato wants to merge 1 commit into
Rust-GPU:mainfrom
LegNeato:issue615-usize-u32

Conversation

@LegNeato

Copy link
Copy Markdown
Collaborator

Fixes #615.

After the nightly-2026-05-22 upgrade, TryFromIntError is no longer a ZST, so Result<u32, TryFromIntError> is laid out as BackendRepr::Memory instead of ScalarPair. Multi-variant enums with this layout were translated to a tag-only OpTypeStruct, so payload field accesses couldn't be recovered into a valid OpAccessChain.

The SPIR-V type now also includes each variant's non-ZST payload fields at their absolute offsets, so payload accesses work.

Fixes Rust-GPU#615.

After the nightly-2026-05-22 upgrade, `TryFromIntError` is no longer a
ZST, so `Result<u32, TryFromIntError>` is laid out as `BackendRepr::Memory`
instead of `ScalarPair`. Multi-variant enums with this layout were
translated to a tag-only `OpTypeStruct`, so payload field accesses
couldn't be recovered into a valid `OpAccessChain`.

The SPIR-V type now also includes each variant's non-ZST payload fields
at their absolute offsets, so payload accesses work.
@LegNeato LegNeato force-pushed the issue615-usize-u32 branch from 138d04a to dfdd6e0 Compare June 14, 2026 04:04

@nazar-pc nazar-pc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can confirm that this one works

nazar-pc added a commit to nazar-pc/abundance that referenced this pull request Jun 14, 2026
@tuguzT

tuguzT commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Yes, this PR fixes compilation issue, but requires usage of OpCapability Int8 to be actually compiled (for some unknown reason); without capability it produces such message:

  error: `u8` type used without `OpCapability Int8`
      |
      = note: used by `core::result::Result<usize, core::num::TryFromIntError>`
  note: used from within `<usize as core::convert::TryFrom<u32>>::try_from`
     --> <***\.rustup\toolchains\nightly-2026-05-22-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\convert\num.rs>:463:5
      |
  463 |     rev!(impl_try_from_unbounded, usize => u32);
      |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  note: called by `<u32 as core::convert::TryInto<usize>>::try_into`
     --> <***\.rustup\toolchains\nightly-2026-05-22-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\convert\mod.rs>:820:9
      |
  820 |         U::try_from(self)
      |         ^^^^^^^^^^^^^^^^^

I guess this is because of blanket implementation could not be inlined (or something similar to that)?

@nazar-pc

Copy link
Copy Markdown
Contributor

Not in my case, I guess it depends on the code

@LegNeato

Copy link
Copy Markdown
Collaborator Author

Hmmm, yeah it depends on the code. TryFromIntError is actually a u8 under the covers. This has come up before in a different context, can't remember what we did so I'll poke around.

@LegNeato

LegNeato commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author

Ok, we don't currently handle this automatically in any way and just punt to the user AFAICT. That's probably correct, it would be weird to promote to u32 and also weird to add a capability silently. We also can't DCE within a type defintion.

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.

Regression: usize to u32 conversion could not be compiled now

3 participants