Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions crates/rustc_codegen_spirv/src/builder/builder_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ use std::ops::{BitAnd, BitOr, BitXor, Not, RangeInclusive};
use tracing::{Level, instrument, span};
use tracing::{trace, warn};

const LARGE_ARRAY_MEMSET_WARN_THRESHOLD: usize = 1024;

enum ConstValue {
Unsigned(u128),
Signed(i128),
Expand Down Expand Up @@ -425,8 +427,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.def(self)
}
SpirvType::Array { element, count } => {
if fill_byte == 0 {
return self.constant_null(ty.def(self.span(), self)).def(self);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context (SPIR-T)

(Future) SPIR-T supports OpConstantNull (e.g. in Rust-GPU/spirt#45) so this is definitely a good catch and won't cause any issues!

I haven't implemented SPV_EXT_replicated_composites (and VK_EXT_shader_replicated_composites has pretty bad adoption - according to https://vulkan.gpuinfo.org/listextensions.php - not to mention we'd need user opt-in anyway).

However, SPIR-T could take OpConstantCompositeReplicateEXT as input, and specifically Rust-GPU/spirt#45 would map it to either undoing the optimization, or optionally re-emitting OpConstantCompositeReplicateEXT on the output side if the extension is enabled etc. (and internally SPIR-T could optimize for this situation, but it doesn't really depend on how SPIR-V does any of this).


What else you should/could do here:

  • because OpConstantNull supports all of these types, you can go as far as making memset_const_pattern return OpConstantNull regardless of the type, and not keeping it specific to arrays (and it would also support e.g. structs)
  • (optional) for full SpirvType::Adt support in memset_const_pattern you just need to iterate over field types
  • (optional) I'd almost want to say that constant_composite itself should check for this situation

}
let elem_pat = self.memset_const_pattern(&self.lookup_type(element), fill_byte);
let count = self.builder.lookup_const_scalar(count).unwrap() as usize;
if count > LARGE_ARRAY_MEMSET_WARN_THRESHOLD {
self.warn(format!(
"large array of {count} elements with a non-zero fill will generate a \
SPIR-V constant with {count} operands, which may be slow to compile; \
consider using a storage buffer for large data instead"
));
}
self.constant_composite(ty.def(self.span(), self), iter::repeat_n(elem_pat, count))
.def(self)
}
Expand Down Expand Up @@ -1928,6 +1940,13 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
count: u64,
dest: PlaceRef<'tcx, Self::Value>,
) {
if count > LARGE_ARRAY_MEMSET_WARN_THRESHOLD as u64 {
self.warn(format!(
"large array of {count} elements with a non-zero fill will generate {count} \
SPIR-V store instructions, which may be slow to compile; \
consider using a storage buffer for large data instead"
));
}
let zero = self.const_usize(0);
let start = dest.project_index(self, zero).val.llval;

Expand Down
9 changes: 9 additions & 0 deletions crates/rustc_codegen_spirv/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

#[track_caller]
pub fn warn(&self, msg: impl Into<DiagMessage>) {
if let Some(current_span) = self.current_span {
self.tcx.dcx().span_warn(current_span, msg);
} else {
self.tcx.dcx().warn(msg);
}
}

#[track_caller]
pub fn err(&self, msg: impl Into<DiagMessage>) {
if let Some(current_span) = self.current_span {
Expand Down
17 changes: 17 additions & 0 deletions tests/compiletests/ui/lang/consts/large_array_nonzero_fill.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Tests that large non-zero-initialized arrays emit a compile warning.
// A `[v; N]` array with non-zero v generates N SPIR-V instructions (either
// store ops or OpConstantComposite operands), which is inherently slow.

// build-pass

#![no_std]
use spirv_std::glam::Vec4;
use spirv_std::spirv;

#[spirv(vertex)]
pub fn test_vs(#[spirv(push_constant)] index: &u32, #[spirv(position)] out_pos: &mut Vec4) {
let nonzeroed = [1.0f32; 2048];

let i = *index as usize % 2048;
*out_pos = Vec4::new(nonzeroed[i], nonzeroed[i], 0.0, 1.0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
warning: large array of 2048 elements with a non-zero fill will generate 2048 SPIR-V store instructions, which may be slow to compile; consider using a storage buffer for large data instead
--> $DIR/large_array_nonzero_fill.rs:13:21
|
LL | let nonzeroed = [1.0f32; 2048];
| ^^^^^^^^^^^^^^

warning: 1 warning emitted

17 changes: 17 additions & 0 deletions tests/compiletests/ui/lang/consts/large_array_zero_fill.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Tests that zero-initialized large arrays compile without warnings.
// A `[0; N]` array goes through `memset_const_pattern` with fill_byte=0,
// which should emit a single OpConstantNull

// build-pass

#![no_std]
use spirv_std::glam::Vec4;
use spirv_std::spirv;

#[spirv(vertex)]
pub fn test_vs(#[spirv(push_constant)] index: &u32, #[spirv(position)] out_pos: &mut Vec4) {
let zeroed = [0.0f32; 2048];

let i = *index as usize % 2048;
*out_pos = Vec4::new(zeroed[i], zeroed[i], 0.0, 1.0);
}
Loading