eddyb ( Eduard-Mihai Burtescu )

Reviewers

rustup: update to `nightly-2023-05-27`.

Learned my lesson with 0.8, so let's get this out of the way early in the 0.9 release cycle. <sub>(And yes, there was panic!-related nonsense to deal with this time too: if cfg!(debug_assertions) not being optimized out)</sub>

Leaving as draft until #1070 lands, just to keep testing simpler (though it's not that critical of an issue).

+184 -125

1 commit in LykenSol/rust-gpu/tree/rustup EmbarkStudios/rust-gpu/tree/main

0 comment

38 changed files

pr created time in 5 hours

create barnchLykenSol/rust-gpu

branch : rustup

created branch time in 5 hours

PR opened EmbarkStudios/rust-gpu

Lower aborts (incl. panics) to "return from entry-point", instead of infinite loops.

We currently map the abort intrinsic (used almost exclusively for panic) to infinite loops, and they either:

are optimized out by spirv-opt or drivers (i.e. treated as UB)
- this obviously changes the semantics, and if a conditional panic protected against some UB, now that UB is unconditional and could affect more code
- worst case, unwanted side-effects could run (e.g. writes of corrupted values, to buffers)
are preserved by both spirv-opt and drivers, and cause a timeout when used
- see these previous discussions for what it take to intentionally cause this:
  - https://github.com/EmbarkStudios/rust-gpu/issues/1048
  - https://github.com/EmbarkStudios/rust-gpu/pull/1055
- however, this is only a reliable option cross-hardware only for compute-only Vulkan queues (not just compute shaders, but compute shaders on a non-graphical queue), where the timeout can't block other work
- worst case, one can easily accidentally hang their entire desktop if GPU compositing is involved
  - (yes I've done this to myself while trying out the barrier trick, no it wasn't fun)

With infinite loops being so terrible, I propose we move towards a "well-defined invocation exit" approach, where we keep the "abort" as a custom instruction (using our "extended instruction set", added in #1064), and then effectively emulate the semantics of OpTerminateInvocation for it by:

inlining any function that uses our custom Abort (either directly, or transitively through some functions it calls) - in the end, we should end up with Aborts only used directly from entry-points
in entry-points, we rewrite Aborts to a plain OpReturn (from the entry-point, i.e. exiting the invocation)
- we could potentially have a mode where we generate a debugPrintf call at this point, with the same inlining-aware "backtrace" we use elsewhere, so that the user gets some feedback if they have the validation layers enabled (and/or try to extract a panic message when we generate the abort in the first place, too)

This PR implements that proposal (but without any debugPrintf conveniences), and so far it seems to work great, but I haven't tested the performance impact (i.e. where before the infinite loops were optimized away, now we're seeing an actual cost to various e.g. bounds checks, that need to do something at all).

There are also other ways of implementing this, and we could do the Abort -> OpReturn rewriting very late (if we think it would be better than letting the SPIR-T structurizer see it), so there's some room to explore mitigations to perf issues, if they arise.

(I will leave this PR as draft until we're sure about the perf aspects)

+270 -59

1 commit in LykenSol/rust-gpu/tree/custom-abort EmbarkStudios/rust-gpu/tree/main

0 comment

11 changed files

pr created time in 7 hours

create barnchLykenSol/rust-gpu

branch : custom-abort

created branch time in 7 hours

push eventLykenSol/rust-gpu

Eduard-Mihai Burtescu

commit sha d35fa4219b41092bf2e4722ad18976331df2b1bf

Update `minifb` dependency.