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 11, 2024
1 parent 1932353 commit 58d0b23
Show file tree
Hide file tree
Showing 11 changed files with 98 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 @@ -649,9 +649,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 @@ -406,7 +406,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 @@ -528,8 +529,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 @@ -566,6 +572,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_u64(count) == Some(0) =>
{
self.fatal(format!(
"Cannot index into [{}; 0], will panic at runtime.",
self.debug_type(element)
))
}
SpirvType::Array { element, .. } | SpirvType::RuntimeArray { element } => {
element
}
Expand Down
4 changes: 4 additions & 0 deletions crates/rustc_codegen_spirv/src/codegen_cx/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,10 @@ impl<'tcx> CodegenCx<'tcx> {
}
self.constant_composite(ty, values.into_iter())
}
SpirvType::Array { count, .. } if self.builder.lookup_const_u64(count) == Some(0) =>
{
self.undef(ty)
}
SpirvType::Array { element, count } => {
let count = self.builder.lookup_const_u64(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 @@ -368,6 +368,9 @@ impl SpirvType<'_> {
.bytes(),
)
.expect("alignof: Vectors must have power-of-2 size"),
Self::Array { count, .. } if cx.builder.lookup_const_u64(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
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: Cannot index into [i32; 0], will panic at runtime.
--> $DIR/array_0.rs:10:13
|
10 | let x = array[0];
| ^^^^^^^^

error: aborting due to 1 previous error

11 changes: 11 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,11 @@
// 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] = ();
}
}
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: Cannot index into [u32; 0], will panic at runtime.
--> $DIR/gep0.rs:9:9
|
9 | array[i] += i;
| ^^^^^^^^^^^^^

error: aborting due to 1 previous error

10 changes: 10 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,10 @@
// build-fail

#![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];
array[0] = 1;
}
10 changes: 10 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,10 @@
error: this operation will panic at runtime
--> $DIR/oob_0_array.rs:9:5
|
9 | array[0] = 1;
| ^^^^^^^^ 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

0 comments on commit 58d0b23

Please sign in to comment.