Improve GPU codegen by defaulting to immediate-abort#157834
Conversation
|
These commits modify compiler targets. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Hm, I see the point of not wanting to pay for panicking. On the other hand, I did use panic handlers. It’s nice to get an error message if an |
|
Hm, would you be open to setting |
|
Could we have abort for debug mode and immediate-abort for release mode? Edit: So, with debug-assertions you would get abort and without it would be immediate-abort. |
|
Not easily. If any crate is compiled with panic=immediate-abort, then all crates must be compiled with it, including the standard library. In fact panic=immediate-abort mostly works by swapping out some functions in libcore. |
^ Exactly this. It's unfortunate, that specifying it by hand is currently not so easy. Having
I wouldn't consider the necessity to recompile everything a problem for our use case. For amdgcn and NVPTX we have to do this anyway to produce correct code. To me it looks like this is mostly a build system problem since the Cargo flags required to do that are experimental and therefore are not nicely integrated into it. |
When looking at the IR of some GPU benchmarks, we noticed that even with panic=abort we ended up with quite a lot of extra IR from the panic machinery. With
panic=immediate-abort, those become simple branches to a trap call:We mostly see it being generated from bounds checks.
Specifying immediate-abort by hand is quite annoying.
cargo clean.-Zbuild-std=core,panic_abortThe only benefit of abort over immediate-abort is the ability to hook in your own panic handler. I think that's a rarely needed feature on GPUs, so we should prioritize better codegen, especially if it's so annoying to specify by hand otherwise.
@Flakebi @kjetilkjeka @kulst are you ok with this as target maintainers?
r? @saethlin