From d569771bb0de001a296f322f6fbb5184155f550c Mon Sep 17 00:00:00 2001 From: julianknodt Date: Sat, 7 Dec 2024 00:16:20 -0800 Subject: [PATCH] Fix zero-sized Array GEPs Previously GEP on zero-sized arrays, would fail. This change fixes arrays to instead emit runtime arrays. I do not know if this will lead to any runtime cost, but it fixes all the compile errors. --- crates/rustc_codegen_spirv/src/abi.rs | 3 --- .../src/builder/builder_methods.rs | 20 ++++++++++++++++--- .../src/codegen_cx/constant.rs | 5 +++++ crates/rustc_codegen_spirv/src/spirv_type.rs | 3 +++ tests/README.md | 2 ++ tests/ui/lang/core/array/array_0.rs | 11 ++++++++++ tests/ui/lang/core/array/array_0.stderr | 8 ++++++++ tests/ui/lang/core/array/array_entry_param.rs | 9 +++++++++ .../lang/core/array/array_entry_param.stderr | 8 ++++++++ tests/ui/lang/core/array/array_zst_0.rs | 12 +++++++++++ tests/ui/lang/core/array/array_zst_0.stderr | 10 ++++++++++ tests/ui/lang/core/array/gep0.rs | 16 +++++++++++++++ tests/ui/lang/core/array/gep0.stderr | 8 ++++++++ tests/ui/lang/core/array/oob_0_array.rs | 16 +++++++++++++++ tests/ui/lang/core/array/oob_0_array.stderr | 14 +++++++++++++ tests/ui/lang/core/array/unused_array_0.rs | 7 +++++++ 16 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lang/core/array/array_0.rs create mode 100644 tests/ui/lang/core/array/array_0.stderr create mode 100644 tests/ui/lang/core/array/array_entry_param.rs create mode 100644 tests/ui/lang/core/array/array_entry_param.stderr create mode 100644 tests/ui/lang/core/array/array_zst_0.rs create mode 100644 tests/ui/lang/core/array/array_zst_0.stderr create mode 100644 tests/ui/lang/core/array/gep0.rs create mode 100644 tests/ui/lang/core/array/gep0.stderr create mode 100644 tests/ui/lang/core/array/oob_0_array.rs create mode 100644 tests/ui/lang/core/array/oob_0_array.stderr create mode 100644 tests/ui/lang/core/array/unused_array_0.rs diff --git a/crates/rustc_codegen_spirv/src/abi.rs b/crates/rustc_codegen_spirv/src/abi.rs index 04d1bf9860..d7499d2f2d 100644 --- a/crates/rustc_codegen_spirv/src/abi.rs +++ b/crates/rustc_codegen_spirv/src/abi.rs @@ -745,9 +745,6 @@ fn trans_aggregate<'tcx>(cx: &CodegenCx<'tcx>, span: Span, ty: TyAndLayout<'tcx> element: element_type, } .def(span, cx) - } else if count == 0 { - // spir-v doesn't support zero-sized arrays - create_zst(cx, span, ty) } else { let count_const = cx.constant_u32(span, count as u32); let element_spv = cx.lookup_type(element_type); diff --git a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs index 7e61c1b989..2f4a54e6a0 100644 --- a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs +++ b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs @@ -409,7 +409,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let ptr = ptr.strip_ptrcasts(); let mut leaf_ty = match self.lookup_type(ptr.ty) { SpirvType::Pointer { pointee } => pointee, - other => self.fatal(format!("non-pointer type: {other:?}")), + SpirvType::Adt { .. } => return None, + other => self.fatal(format!("adjust_pointer for non-pointer type: {other:?}")), }; // FIXME(eddyb) this isn't efficient, `recover_access_chain_from_offset` @@ -531,8 +532,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let stride = ty_kind.sizeof(self)?; ty_size = MaybeSized::Sized(stride); - indices.push((offset.bytes() / stride.bytes()).try_into().ok()?); - offset = Size::from_bytes(offset.bytes() % stride.bytes()); + if stride.bytes() == 0 { + indices.push(0); + offset = Size::from_bytes(0); + } else { + indices.push((offset.bytes() / stride.bytes()).try_into().ok()?); + offset = Size::from_bytes(offset.bytes() % stride.bytes()); + } } _ => return None, } @@ -569,6 +575,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .iter() .map(|index| { result_pointee_type = match self.lookup_type(result_pointee_type) { + SpirvType::Array { count, element } + if self.builder.lookup_const_scalar(count) == Some(0) => + { + self.fatal(format!( + "evaluation of constant value failed: cannot index into [{}; 0]", + self.debug_type(element) + )) + } SpirvType::Array { element, .. } | SpirvType::RuntimeArray { element } => { element } diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs b/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs index 162eb9dc8c..2a627a42c5 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs @@ -480,6 +480,11 @@ impl<'tcx> CodegenCx<'tcx> { } self.constant_composite(ty, values.into_iter()) } + SpirvType::Array { count, .. } + if self.builder.lookup_const_scalar(count) == Some(0) => + { + self.undef(ty) + } SpirvType::Array { element, count } => { let count = self.builder.lookup_const_scalar(count).unwrap() as usize; let values = (0..count).map(|_| { diff --git a/crates/rustc_codegen_spirv/src/spirv_type.rs b/crates/rustc_codegen_spirv/src/spirv_type.rs index 0c8ce42ba0..1265d8372d 100644 --- a/crates/rustc_codegen_spirv/src/spirv_type.rs +++ b/crates/rustc_codegen_spirv/src/spirv_type.rs @@ -373,6 +373,9 @@ impl SpirvType<'_> { .bytes(), ) .expect("alignof: Vectors must have power-of-2 size"), + Self::Array { count, .. } if cx.builder.lookup_const_scalar(count) == Some(0) => { + Align::from_bytes(0).unwrap() + } Self::Array { element, .. } | Self::RuntimeArray { element } | Self::Matrix { element, .. } => cx.lookup_type(element).alignof(cx), diff --git a/tests/README.md b/tests/README.md index 1a3abc95b3..0ca83873d8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -21,3 +21,5 @@ the resulting binary with spirv-val). Because of this, there might be some stran the point isn't to make a fully functional shader every time (that would take an annoying amount of effort), but rather validate that specific parts of the compiler are doing their job correctly (either succeeding as they should, or erroring as they should). + +For more docs on compiletests, check the [rustc docs](https://rustc-dev-guide.rust-lang.org/tests/compiletest.html). diff --git a/tests/ui/lang/core/array/array_0.rs b/tests/ui/lang/core/array/array_0.rs new file mode 100644 index 0000000000..dad076b490 --- /dev/null +++ b/tests/ui/lang/core/array/array_0.rs @@ -0,0 +1,11 @@ +#![allow(unconditional_panic)] + +// build-fail +#![cfg_attr(target_arch = "spirv", no_std)] +use spirv_std::spirv; + +#[spirv(compute(threads(1, 1, 1)))] +pub fn compute() { + let array = [0; 0]; + let x = array[0]; +} diff --git a/tests/ui/lang/core/array/array_0.stderr b/tests/ui/lang/core/array/array_0.stderr new file mode 100644 index 0000000000..ad672b6d50 --- /dev/null +++ b/tests/ui/lang/core/array/array_0.stderr @@ -0,0 +1,8 @@ +error: evaluation of constant value failed: cannot index into [i32; 0] + --> $DIR/array_0.rs:10:13 + | +10 | let x = array[0]; + | ^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/lang/core/array/array_entry_param.rs b/tests/ui/lang/core/array/array_entry_param.rs new file mode 100644 index 0000000000..01c6dd341b --- /dev/null +++ b/tests/ui/lang/core/array/array_entry_param.rs @@ -0,0 +1,9 @@ +// build-fail +#![allow(unconditional_panic)] + +#![cfg_attr(target_arch = "spirv", no_std)] +use spirv_std::spirv; + +// note that &mut [usize; 0] will cause an even worse panic +#[spirv(compute(threads(1, 1, 1)))] +pub fn compute(m: [usize; 0]) {} diff --git a/tests/ui/lang/core/array/array_entry_param.stderr b/tests/ui/lang/core/array/array_entry_param.stderr new file mode 100644 index 0000000000..3145d8b82b --- /dev/null +++ b/tests/ui/lang/core/array/array_entry_param.stderr @@ -0,0 +1,8 @@ +error: entry point parameter type not yet supported (`[usize; 0]` has size `0`) + --> $DIR/array_entry_param.rs:9:19 + | +9 | pub fn compute(m: [usize; 0]) {} + | ^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/lang/core/array/array_zst_0.rs b/tests/ui/lang/core/array/array_zst_0.rs new file mode 100644 index 0000000000..3e957b0317 --- /dev/null +++ b/tests/ui/lang/core/array/array_zst_0.rs @@ -0,0 +1,12 @@ +// build-pass +#![cfg_attr(target_arch = "spirv", no_std)] +use spirv_std::spirv; + +#[spirv(compute(threads(1, 1, 1)))] +pub fn compute() { + let mut array = [(); 0]; + for i in 0..array.len() { + array[i] = (); + } + let () = array[0]; +} diff --git a/tests/ui/lang/core/array/array_zst_0.stderr b/tests/ui/lang/core/array/array_zst_0.stderr new file mode 100644 index 0000000000..7981745aca --- /dev/null +++ b/tests/ui/lang/core/array/array_zst_0.stderr @@ -0,0 +1,10 @@ +error: this operation will panic at runtime + --> $DIR/array_zst_0.rs:11:14 + | +11 | let () = array[0]; + | ^^^^^^^^ index out of bounds: the length is 0 but the index is 0 + | + = note: `#[deny(unconditional_panic)]` on by default + +error: aborting due to 1 previous error + diff --git a/tests/ui/lang/core/array/gep0.rs b/tests/ui/lang/core/array/gep0.rs new file mode 100644 index 0000000000..8c87b0e738 --- /dev/null +++ b/tests/ui/lang/core/array/gep0.rs @@ -0,0 +1,16 @@ +// build-fail + +#![cfg_attr(target_arch = "spirv", no_std)] +use spirv_std::spirv; + +fn example() { + let mut array = [0; N]; + for i in 0..N { + array[i] += i; + } +} + +#[spirv(compute(threads(1, 1, 1)))] +pub fn compute() { + example::<0>(); +} diff --git a/tests/ui/lang/core/array/gep0.stderr b/tests/ui/lang/core/array/gep0.stderr new file mode 100644 index 0000000000..dd0d6b09b6 --- /dev/null +++ b/tests/ui/lang/core/array/gep0.stderr @@ -0,0 +1,8 @@ +error: evaluation of constant value failed: cannot index into [u32; 0] + --> $DIR/gep0.rs:9:9 + | +9 | array[i] += i; + | ^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/lang/core/array/oob_0_array.rs b/tests/ui/lang/core/array/oob_0_array.rs new file mode 100644 index 0000000000..50b3e9cf18 --- /dev/null +++ b/tests/ui/lang/core/array/oob_0_array.rs @@ -0,0 +1,16 @@ +// build-pass +// compile-flags: -C llvm-args=--disassemble-entry=compute +#![allow(unconditional_panic)] + +#![cfg_attr(target_arch = "spirv", no_std)] +use spirv_std::spirv; + +#[spirv(compute(threads(1, 1, 1)))] +pub fn compute() { + let mut array = [0; 0]; + // writes to an array compile fine although they should be a panic. + // (&mut array)[0] = 1; // this fails to compile, but it seems that below is + // optimized out, and I'm not sure where. + // GEP is not being hit, and neither is any load/store. + array[0] = 1; +} diff --git a/tests/ui/lang/core/array/oob_0_array.stderr b/tests/ui/lang/core/array/oob_0_array.stderr new file mode 100644 index 0000000000..9b7934efb5 --- /dev/null +++ b/tests/ui/lang/core/array/oob_0_array.stderr @@ -0,0 +1,14 @@ +%1 = OpFunction %2 None %3 +%4 = OpLabel +OpLine %5 15 4 +%6 = OpULessThan %7 %8 %8 +OpNoLine +OpSelectionMerge %9 None +OpBranchConditional %6 %10 %11 +%10 = OpLabel +OpBranch %9 +%11 = OpLabel +OpReturn +%9 = OpLabel +OpReturn +OpFunctionEnd diff --git a/tests/ui/lang/core/array/unused_array_0.rs b/tests/ui/lang/core/array/unused_array_0.rs new file mode 100644 index 0000000000..54976654dd --- /dev/null +++ b/tests/ui/lang/core/array/unused_array_0.rs @@ -0,0 +1,7 @@ +#![cfg_attr(target_arch = "spirv", no_std)] +use spirv_std::spirv; + +#[spirv(compute(threads(1, 1, 1)))] +pub fn compute() { + let array = [0; 0]; +}