Open RalfJung opened 1 year ago
I know this issue is related to wasm. But it's also the one that is linked to when hitting the assert failure when using PassMode::Direct
with abi::Abi::Aggregate
and this is a general question about that.
On the nvptx64
target normal device functions does not hit this assert due to using make_indirect()
for aggregates. This is not possible for functions with the Abi::PtxKernel
abi due to a call not being a proper call but instead involving the copy of the arguments from CPU (and a copied pointer would usually be invalid). I have a few questions so I'm able to understand and resolve this problem.
What kind of details leaks from llvm to the abi when using PassMode::Direct
?
Is PassMode::Cast
to an Uniform with the correct alignment and size the correct one to use? It states that a uniform is based on registers, but I assume that can be very loosely translated to copying the data from CPU to GPU.
EDIT: the solution with PassMode::Cast
ends up breaking the abi tests for extern "ptx-kernel" fn
. This abi is unstable and feature gated so it's luckily not a backward compatibility issue.
@kjetilkjeka please open a new issue; this one here is for the wasm ABI problems.
FWIW, rust-gpu
had to use the new make_direct_deprecated()
: https://github.com/EmbarkStudios/rust-gpu/pull/1109/files#diff-9ff3a35aa9e19911b0f93cebdc627768b3ca62ee19cb2c233c787606df7a4426R67. I don't know enough about either project to know if it is the same issue as this or if it should be fixed on the rust-gpu
side, but I wanted to add a link here for posterity.
Hm yeah that's not great, though it might be related to https://github.com/rust-lang/rust/issues/117271 given both are about GPUs.
Hm yeah that's not great, though it might be related to https://github.com/rust-lang/rust/issues/117271 given both are about GPUs.
I'm reviewing the change @LegNeato mentioned and ended up here semi-randomly, so I might as well answer this:
Rust-GPU only started using PassMode::Direct
:
PassMode::Cast
, as it's transmute
-like
(as it gets into "antiquated GPU typed memory lacking union
s" territory, or at least ptr<->int transmutes - and even with those being theoretically solvable, in the short term it helps to not even have the problem)PassMode::Indirect
as well avoids having to eliminate memory accesses (i.e. via equivalents of LLVM mem2reg+SROA)
(not strictly necessary: calls or even just moving values around already likely introduces this problem intra-function, and any calls with problematic indirection would get inlined anyway)So it probably makes most sense to turn PassMode::Cast
into PassMode::Indirect
instead, anyway (which AFAICT is equivalent to using ArgAbi::new
and just never doing any adjustment logic beyond "ignore ZSTs").
And while writing this comment I went ahead and tried it and the only issues stem from lacking extra logic to handle the indirect case in a few places (mainly the format_args!
"decompiler"), so Rust-GPU wouldn't get impacted by make_direct_deprecated
being removed, beyond a bit of busywork.
(which I'm only punting on in the very short term because of similar-yet-different changes needed to update to newer nightlies etc., but after that I'll be happy to get rid of another hack from Rust-GPU)
rustc has a
PassMode
enum to declare how arguments should be passed between functions.PassMode::Direct
is used for the simple case where we can just generate LLVM IR with an argument type that matches the usual LLVM type for this Rust type. It should only be used for types withScalar
orVector
ABI; for larger types, Rust will generate LLVM struct/union types and thoserepr(transparent)
types.Unfortunately the compiler never had an assertion ensuring that
PassMode::Direct
is only used withScalar
/Vector
types, and so one target accidentally broke this rule: wasm32-unknown-unknown. Later all wasm targets got anextern "wasm"
ABI that has the same issue. We should find some way to fix this target, as the current situation makes life harder for anyone wanting to tweak our Rust-to-LLVM-type-mapping, and for alternative backends that have to match our effective ABI.The issue is that last time @bjorn3 tried to fix this, it lead to https://github.com/rust-lang/rust/issues/81386 and had to be reverted. It will be basically impossible to do this change without breaking the ABI at least for some cases. I'm not sure how much ABI breakage we can get away with. This might have to be blocked on https://github.com/rust-lang/compiler-team/issues/672 which should help to implement the current effective ABI at least for types that are ABI-stable (i.e.,
repr(C)
andrepr(transparent)
).Cc https://github.com/rust-lang/rust/issues/122532