Skip to content

Commit

Permalink
Fix zero-sized Array GEPs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JulianKnodt committed Dec 22, 2024
1 parent 561d2eb commit d569771
Show file tree
Hide file tree
Showing 16 changed files with 146 additions and 6 deletions.
3 changes: 0 additions & 3 deletions crates/rustc_codegen_spirv/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 17 additions & 3 deletions crates/rustc_codegen_spirv/src/builder/builder_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions crates/rustc_codegen_spirv/src/codegen_cx/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|_| {
Expand Down
3 changes: 3 additions & 0 deletions crates/rustc_codegen_spirv/src/spirv_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
11 changes: 11 additions & 0 deletions tests/ui/lang/core/array/array_0.rs
Original file line number Diff line number Diff line change
@@ -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];
}
8 changes: 8 additions & 0 deletions tests/ui/lang/core/array/array_0.stderr
Original file line number Diff line number Diff line change
@@ -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

9 changes: 9 additions & 0 deletions tests/ui/lang/core/array/array_entry_param.rs
Original file line number Diff line number Diff line change
@@ -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]) {}
8 changes: 8 additions & 0 deletions tests/ui/lang/core/array/array_entry_param.stderr
Original file line number Diff line number Diff line change
@@ -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

12 changes: 12 additions & 0 deletions tests/ui/lang/core/array/array_zst_0.rs
Original file line number Diff line number Diff line change
@@ -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];
}
10 changes: 10 additions & 0 deletions tests/ui/lang/core/array/array_zst_0.stderr
Original file line number Diff line number Diff line change
@@ -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

16 changes: 16 additions & 0 deletions tests/ui/lang/core/array/gep0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// build-fail

#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

fn example<const N: usize>() {
let mut array = [0; N];
for i in 0..N {
array[i] += i;
}
}

#[spirv(compute(threads(1, 1, 1)))]
pub fn compute() {
example::<0>();
}
8 changes: 8 additions & 0 deletions tests/ui/lang/core/array/gep0.stderr
Original file line number Diff line number Diff line change
@@ -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

16 changes: 16 additions & 0 deletions tests/ui/lang/core/array/oob_0_array.rs
Original file line number Diff line number Diff line change
@@ -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;
}
14 changes: 14 additions & 0 deletions tests/ui/lang/core/array/oob_0_array.stderr
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions tests/ui/lang/core/array/unused_array_0.rs
Original file line number Diff line number Diff line change
@@ -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];
}

0 comments on commit d569771

Please sign in to comment.