Convert to use glam for math types

[repi]

Removes the temporary math types in spirv-std and uses our experimental spirv fork of glam (bitshifter/glam-rs#85) for our example shader! 🎉

Todo:

• Proper copysign
• Fix constant assert_uninit
• Compile on CPU
• Compile on GPU
• CUT: Solve clippy error: constant runtime array value - CI clippy on shader disabled for now, tracked in Investigate clippy failure on example-shader with glam #186

Fix #134

[bjorn3]

rust-gpu/rustc_codegen_spirv/src/codegen_cx/constant.rs

Line 408 in 75ca545

Self::assert_uninit(alloc, base, *offset, occupied_spaces);

asserts that padding bytes are undef while at the same type padding bytes of contained types are not undef. Neither has to be true.

[khyperia]

Neither has to be true.

This asserting that padding bytes of contained types is def is a bug, but, I'm really surprised that padding bytes are not required to be undef. Why was that decision made?

(And just as a frustrated aside, it sucks that this whole thing even exists, it'd be so nice if I didn't have to reimplement rustc's understanding of interpreting semantics from bytes...)

edit: just debugged this, looks like the failure is indeed due to the bug of asserting that padding of contained types is def. @repi, feel free to delete the assert_uninit function, or I can push a commit doing so if you'd like.

[bjorn3]

This asserting that padding bytes of contained types is def is a bug, but, I'm really surprised that padding bytes are not required to be undef. Why was that decision made?

(And just as a frustrated aside, it sucks that this whole thing even exists, it'd be so nice if I didn't have to reimplement rustc's understanding of interpreting semantics from bytes...)

Currently all values allow defined padding bytes. I think there is work being done to refactor the representation of constants such that padding bytes are always undefined in the result of const eval. This only applies to direct references. This won't apply to raw pointers, as those are defined to not care about the pointee. For example it is perfectly valid to have a *const bool point to 2 for as long as you don't read it. The consensus from rust-lang/unsafe-code-guidelines#77 seems to be that the validity of references won't care about the pointee either. Rust's memory model uses untyped memory, unlike C which uses typed memory. See rust-lang/unsafe-code-guidelines#84 (comment) by @RalfJung for why untyped memory is arguably better:

[...]
Not sure if I'd call it "scary", but my dislike for typed memory stems mostly from experience with strict aliasing in C++: it is a huge pain for programmers (see all the projects using -fno-strict-aliasing and the many more projects that should set this flag) and gives very weak optimizations compared to Stacked Borrows. So I think we just don't need it. Also I think there yet has to be a proposal that integrates this well with a language that allows byte-wise memory access and mutation. (The C object memory model is a huge mess and AFAIK still has not been described precisely. The best approximation I know is in this thesis.)
[...]

[khyperia]

I think there is work being done to refactor the representation of constants such that padding bytes are always undefined in the result of const eval

This is in the context of const eval, though?...

edit: to be specific, all bytes being interpreted here are the result of const eval, even those through pointers. Can non-const-eval bytes somehow leak through here? Where even would those come from?

[RalfJung]

I'm really surprised that padding bytes are not required to be undef. Why was that decision made?

I don't think anyone ever proposed to force them to be undef. That would be a bunch of extra work (and it would require us to precisely define "padding byte" which is not easy for enum). So this is less of a decision that was made and more something that never seemed useful.

What is the benefit to forcing them undef? Note that padding bytes "lose their value" on every typed copy. So for a const, the codegen backend may reset padding. But this is only true for the top-level const value, not for nested allocations, and it is also not true for static where the program can in their observe the value of padding bytes.

[bjorn3]

I don't think anyone ever proposed to force them to be undef.

I was refering to value trees, which I think would omit padding bytes.

[RalfJung]

I was refering to value trees, which I think would omit padding bytes.

Ah yes, those would not preserve padding. But they would also not be used to represent normal const embedded in MIR. Value trees may exist in MIR to represent e.g. integer literals more efficiently, but a normal const/static will be kept as an Allocation for codegen purposes (their value trees will only be used in the type system).

[khyperia]

I was refering to value trees, which I think would omit padding bytes.

I don't know what value trees are, but guessing from the name, it sounds exactly like what we want here. The problem is that spir-v doesn't allow untyped byte array constants to be reinterpreted as typed values (as the LLVM backend does), so we need to walk the byte array with the expected constant type and extract a "typed" constant value (tree) out of it. Is there some way for us to get a value tree from rustc, which I'm guessing is what this is?

[RalfJung]

Value trees are not implemented yet. See rust-lang/compiler-team#323 for some further information. Also, value trees can only represent consts of some types (basically, only integers, structs/tuples/arrays, enums, and references). And finally, passing all const (let alone static) through a value tree is not correct as it will lose padding bytes which could be observable by the program.

The problem is that spir-v doesn't allow untyped byte array constants to be reinterpreted as typed values (as the LLVM backend does), so we need to walk the byte array with the expected constant type and extract a "typed" constant value (tree) out of it.

That sounds then like it is impossible to write a correct rustc backend, I am afraid. Being able to reinterpret data at the byte level is a key ability of Rust. How does the spir-v backend implement raw pointer casts, transmute, or union field accesses?

[bjorn3]

Raw pointer casts for the Logical addressing model, which is used for Vulkan, give an error.

rust-gpu/rustc_codegen_spirv/src/builder_spirv.rs

Line 97 in 7b1a271

builder.memory_model(AddressingModel::Logical, MemoryModel::Vulkan);

#153 improves this error.

transmute seems to be unimplemented. (or cg_ssa is responsible for the impl)

Unions are represented as raw byte arrays with padding initialized to zero.

rust-gpu/rustc_codegen_spirv/src/abi.rs

Lines 556 to 566 in b32c04b

	FieldsShape::Union(_) => {
	assert_ne!(ty.size.bytes(), 0, "{:#?}", ty);
	assert!(!ty.is_unsized(), "{:#?}", ty);
	let byte = SpirvType::Integer(8, false).def(cx);
	let count = cx.constant_u32(ty.size.bytes() as u32);
	SpirvType::Array {
	element: byte,
	count,
	}
	.def(cx)
	}

[khyperia]

That sounds then like it is impossible to write a correct rustc backend, I am afraid. Being able to reinterpret data at the byte level is a key ability of Rust.

Yes, we know, it's crappy to not have the full power of rust available on the GPU, but that's expected, and the benefits of using sort-of-kinda-Rust outweigh being banned from using some parts of the language. (RLSL was called "rust like shading language" for a reason, and this is kind of the spiritual successor to that)

which could be observable by the program

Because we restrict the abilities of Rust, it's probably not observable. Considering this is a two-month-old project that extremely recently moved out of the "one person hacking on it prototype phase", I'm not going to say it definitely isn't observable, but I imagine we'll tighten up and formalize some language restrictions as time goes on.

[RalfJung]

Unions are represented as raw byte arrays with padding initialized to zero.

But then what happens when I store things into a union at one type and read it at another type?

Because we restrict the abilities of Rust, it's probably not observable.

That makes sense, thanks. I was lacking this context when being summoned into this thread. ;)

If you anyway forbid all forms of transmuting (whether they be through unions, raw pointers, and transmute), then those padding bytes are most likely indeed not observable, so you could get away with using a more abstract representation.

That said, not all const can be turned into valtrees -- for example, nothing containing raw pointers or floats, and likely it will also be restricted to types with a "sufficiently well-behaved" PartialEq implementation (this is the "structural match" restriction). And it will only support compile-time values stored in read-only memory (that is crucial for soundness of pattern matching and const generics), so you cannot use it for static either. So, I doubt valtrees are what you'll want to use.

If padding bytes are not observable, then why do you need to assert that they are undefined? That is not something rustc guarantees (and for the reasons discussed above it likely would be incoherent to try to make rustc guarantee this), but if padding bytes are not observable with your backend, you can just ignore them. In fact, "assert that this is undef" is a really strange thing to do -- the entire point of "undef" is that you can replace it by an arbitrary value and everything will still work the same. The assert breaks that key property.

[khyperia]

But then what happens when I store things into a union at one type and read it at another type?

Unions are really sketchily implemented right now and probably don't even work in shader mode (rather than kernel mode), very much an untested prototype thing.

If padding bytes are not observable, then why do you need to assert that they are undefined?

Because I thought it was a reasonable, sane assumption that padding bytes are undefined, and asserting they're undefined helps validate that I'm doing the rather complicated walk over the bytes correctly (especially considering the original TyAndLayout is not accessible, all type structure information must come from the translated-to-backend type, which in the future may differ from what TyAndLayout computed). Obviously that's wrong, so, we're removing the assert, as I mentioned above.

[RalfJung]

especially considering the original TyAndLayout is not accessible, all type structure information must come from the translated-to-backend type, which in the future may differ from what TyAndLayout computed

Why is that inaccessible? MIR should contain the type of the const/static you are accessing, and then you could in principle use our type-based value visitor to traverse that allocation in a type-directed way.

But I am not very familiar with the interface codegen backends work against. Maybe there is some place where more type information could be preserved for backends that need it. (Allocations are entirely untyped, but the places where they are referenced in the MIR should be typed.)

[khyperia]

Why is that inaccessible?

Because there's just literally no TyAndLayout in scope

[RalfJung]

Wherever that scalar is coming from, there should be a Ty, and then you can compute the layout from that. (I am entirely speaking from my experience with Miri here, I have never worked in or on a codegen backend. Also I assume Self::Type does not lead to a rustc Ty.)

[RalfJung]

create_const_alloc seems to have already lost all type information -- as I said, Allocation are indeed untyped, but they are only ever embedded in the MIR in a typed way.

[khyperia]

Wherever that scalar is coming from, there should be a Ty, and then you can compute the layout from that

It's behind a pointer, no? So the ABI is a abi::Primitive::Pointer, which rustc made the decision to make opaque, and so we cannot access the TyAndLayout of the underlying pointee.

[RalfJung]

Not sure why you are concerned with the ABI here. This is what I mean:

const V: Vec<i32> = Vec::new();

fn main() {
    let v = V;
}

has MIR (looking only at the assignment)

        _1 = const V;                    // scope 0 at src/main.rs:4:13: 4:14
                                         // ty::Const
                                         // + ty: std::vec::Vec<i32>
                                         // + val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ playground[8132]::V[0]), const_param_did: None }, [], None)
                                         // mir::Constant
                                         // + span: src/main.rs:4:13: 4:14
                                         // + literal: Const { ty: std::vec::Vec<i32>, val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ playground[8132]::V[0]), const_param_did: None }, [], None) }

Note that ty::Const and mir::Constant are both typed.

[RalfJung]

In the code, for example here you have the layout but do not pass it on:

rust-gpu/rustc_codegen_spirv/src/codegen_cx/constant.rs

Line 320 in 706f4cc

let init = self.create_const_alloc(alloc, ty);

And here you have a DefId which is enough to look up a Ty and then compute its Layout:

rust-gpu/rustc_codegen_spirv/src/codegen_cx/declare.rs

Line 254 in b32c04b

let mut v = self.create_const_alloc(alloc, value_ty);

[khyperia]

the ABI (rustc_target::abi::Layout) is the layout of the bytes, no? so we need the ABI to extract the constant from the byte array.

What about the callsite that I linked earlier? I don't see any way to grab a Ty from there. Sure, there's a Ty for the pointer, but not the pointee.

Also, would it be possible for you to join the discord, instead of discussing on this issue? Totally fine if not, just figured it might be a little easier.

[RalfJung]

I'm not a big fan of discord, but I am on the Rust Zulip (at least on week-ends). :)

What about the callsite that I linked earlier? I don't see any way to grab a Ty from there. Sure, there's a Ty for the pointer, but not the pointee.

If you do a type-based traversal the way this code does (or find a way to use that code), you will have a type here. It is true that Layout is shallow; it stops at pointers (and even at fields of an aggregate, but you seem to be able to handle that). So you'll need to recursively keep a TyAndLayout around, not just a Layout.

See for example this code for a visitor that handles references in a type-driven way.

[repi]

Removed assert_uninit, removed in glam any usage of core::fmt, switched to use the new implement core::copysignf32 intrinsic + some smaller fixes.

And now the shader builds and runs, though is just black in the GPU version :) So must be some miss-compilation or incorrect feature implementation. The CPU version example-runner-cpu with the same glam code does work.

But very nice to get to a buildable and runnable state!

[repi]

this is a weird clippy error on the glam shader:

     Compiling example-shader v0.1.0 (/home/runner/work/rust-gpu/rust-gpu/examples/example-shader)
  error: constant runtime array value

https://github.com/EmbarkStudios/rust-gpu/pull/149/checks?check_run_id=1326417262

it is from our codegen

[repi]

With some additional glam fixes and workarounds, got this compiling and running correctly on both GPU and CPU now! 🎉

Remaining thing to figure out though before merging is clippy on the shader and why that is failing. Or disable clippy on the shaders for now

[repi]

@khyperia @Jasper-Bekkers have a look at this now, passes CI now when I disabled clippy on the shader (for now), and think should be good to get in!