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

Update to nightly-2023-08-29 #1091

Merged
merged 8 commits into from
Nov 21, 2023
Merged

Update to nightly-2023-08-29 #1091

merged 8 commits into from
Nov 21, 2023

Conversation

LegNeato
Copy link
Contributor

@LegNeato LegNeato commented Aug 30, 2023

EDIT(@eddyb): initial PR by @LegNeato but I added to it to avoid creating another PR (#1091 (commits)).

@LegNeato
Copy link
Contributor Author

This builds successfully for me locally but panics when using GPU stuff (CPU example works fine). I have never touched the compiler or this project so I don't really know what I am doing 😆

@LegNeato
Copy link
Contributor Author

LegNeato commented Aug 30, 2023

I'm fairly sure the problems are in the type pointer stuff, changed in rust-lang/rust@04303cf

@LegNeato
Copy link
Contributor Author

I managed to get it further along by adding let ptr = self.pointercast(ptr, self.type_ptr_to(ty));
to various places. It looks like the above change removes those before calling things like load and store.

@LegNeato
Copy link
Contributor Author

I also needed to add a new intrinsic compare_bytes due to rust-lang/rust@502af03.

@LegNeato
Copy link
Contributor Author

LegNeato commented Sep 1, 2023

When I don't add the pointer casts, it compiles but I get:

TODO: intcast not implemented yet: val=SpirvValue { kind: Def(14981), ty: 54 } val.ty=Adt { def_id: Some(DefId(0:34076 ~ core[e34c]::option::Option)), align: Align(4 bytes), size: Some(Size(8 bytes)), field_types: [4, 4], field_offsets: [Size(0 bytes), Size(4 bytes)], field_names: None } dest_ty=Integer(32, true) is_signed=false

When I do, it gets all the way to compiling the shader but bails on converting a *void to a *i32 IIRC.

@eddyb eddyb marked this pull request as ready for review November 21, 2023 16:04
@eddyb eddyb self-requested a review as a code owner November 21, 2023 16:04
@eddyb
Copy link
Contributor

eddyb commented Nov 21, 2023

Hi, sorry to barge in with those commits, but I went ahead and finished this PR - it turned out to be pretty nasty and I'm not that happy with my approach (it's now conservative when creating pointers, but aggressive when accessing them).

So I really hope you didn't waste too much time trying to make it all work (also, feel free to poke me on Discord etc. any time, esp. if you want any feedback, I tend to mainly get GH notifs for a direct ping, i.e. @eddyb).

@eddyb eddyb enabled auto-merge (rebase) November 21, 2023 16:20
@eddyb eddyb merged commit 145a98d into EmbarkStudios:main Nov 21, 2023
7 checks passed
@LegNeato
Copy link
Contributor Author

Thanks completing this! I like to dive into the deep end in codebases to try to learn them, so the time I spent poking at this was time well spent. Doubly so now that I've seen your changes and can compare them to the mental model I built up.

@LegNeato LegNeato deleted the update_nightly branch November 22, 2023 12:32
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