14:04 MrCooper: eric_engestrom: thanks for assigning Marge to the right field! :)
14:22 eric_engestrom: MrCooper: haha sure! I have https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/?reviewer_username=marge-bot bookmarked ever since that gitlab web ui update a couple months ago made everyone start to misclick ^^
14:26 MrCooper: wow, thanks for that
19:32 alyssa: hakzsam: I don't see how nir_lower_abort() can be correct
19:33 alyssa: nir_jump_halt is documented as:
19:33 alyssa: * This instruction is roughly the equivalent of C's "exit()" in that it
19:33 alyssa: * immediately terminates the current shader invocation.
19:33 alyssa: crucially - it only applies to a single shader invocation
19:33 alyssa: not the whole device, not even the whole subgroup
19:34 alyssa: whereas spirv documents
19:34 alyssa: Ceases all further processing in the invocation that executes it. Only instructions executed before OpAbortKHR will have side effects. Other invocations in the same Device scope instance will also be terminated in finite time, though may reach completion
19:34 alyssa: consider a shader doing something like
19:34 alyssa: if (lane == 0) { abort(); } else { for (;;) { /* hang */ } }
19:35 alyssa: spir-v spec guarantees that we eventually terminate, nir documentation does not give that guarantee on the lowered code
19:38 alyssa: nir_jump_halt is - as the name suggests - just a branch. complicating matters, aco implements it with s_sethalt instead of a jump.
19:39 alyssa: which might have the right semantics for the SPIR-V op (the AMD ISA docs are not illuminating tbh), but if they do it's the wrong semantics for nir
19:40 alyssa: (It should be possible to lower @terminate to @demote followed by nir_jump_halt and have everything work. Under NIR rules it does.)
19:41 alyssa: Maybe I'm missing something?
19:42 alyssa: Further confusing matters is that Intel's "HALT" instruction is neither a NIR halt nor a SPIR-V abort, and is instead basically a bad goto.
19:42 alyssa: dj-death: aswar002 ^^
19:47 pixelcluster: alyssa: s_sethalt indeed does have the correct semantics for SPIR-V abort - it's a scalar (s_) instruction so it ignores any active EXEC mask, and instead immediately ceases processing for the entire wavefront when encountered
19:48 alyssa: pixelcluster: i'm not sure that's strong enough? what about like
19:48 alyssa: if (gl_GlobalInvocationID == 10000) { abort(); } else { for(;;); }
19:48 pixelcluster: well, s_sethalt alone indeed does not provide enough guarantees there
19:48 alyssa: provided you have forward progress guarantees (does vulkan have those in $current_year?), my reading of the spec is that shouldn't hang
19:49 alyssa: so s_sethalt is too strong for nir_jump_halt and not strong enough for SPIR-V abort :clown:
19:49 pixelcluster: there isn't really anything the hw provides that could give cross-workgroup guarantees there
19:49 alyssa: is the spec just bogus?
19:49 pixelcluster: however, we kind of utilize the fact that the kernel will, in practice, not execute a submission infinitely long (aka job times out and gets killed by kernel)
19:50 alyssa: yeah ok
19:50 pixelcluster: "ceases processing" also means "hangs indefinitely" btw
19:50 alyssa: heh
19:51 pixelcluster: the kernel really just kinda the only device-wide killer we have
19:51 alyssa: Yea, that's fine I guess
19:51 alyssa: but it doesn't fix the fact that aco is broken, translating nir_jump_halt to something that's not that
19:51 pixelcluster: yup, that's broken indeed
19:52 pixelcluster: it should be an intrinsic
19:52 pixelcluster: not lowered to a jump
19:52 alyssa: Ack
20:04 alyssa: pixelcluster: I don't suppose you're volunteering to fix this haystack on radv/aco?
20:04 alyssa: I would but apparently I don't work on that hardware or something idk
20:20 pixelcluster: alyssa: i can do it tomorrow
20:22 alyssa: thx
21:15 glehmann: I mean, adding another jump type instead of an intrinsic would also be fine
21:15 glehmann: but it really doesn't matter in the end because execution ends
21:19 dj-death: alyssa: might need to do a pagefault for it
21:48 alyssa: dj-death: I was thinking of executing `illegal`
21:48 alyssa: it's technically UB according to the bspec but it should work :P
21:48 alyssa: can't be predicated though so, yeah, page fault might be most effective.
21:55 dj-death: pagefaulting would be a bit sad
21:55 dj-death: because i915 can't ...
21:58 alyssa: i915 is dead :)
22:12 CounterPillow: god I wish