attr: fix #[spirv(flat)] checking to match the Vulkan spec.

This commit is contained in:
Eduard-Mihai Burtescu 2022-10-25 02:13:48 +03:00 committed by Eduard-Mihai Burtescu
parent 097ba40bed
commit 65f892eb50
24 changed files with 116 additions and 69 deletions

View File

@ -1,6 +1,6 @@
use super::CodegenCx;
use crate::abi::ConvSpirvType;
use crate::attr::{AggregatedSpirvAttributes, Entry};
use crate::attr::{AggregatedSpirvAttributes, Entry, Spanned};
use crate::builder::Builder;
use crate::builder_spirv::{SpirvValue, SpirvValueExt};
use crate::spirv_type::SpirvType;
@ -144,6 +144,7 @@ impl<'tcx> CodegenCx<'tcx> {
for (entry_arg_abi, hir_param) in arg_abis.iter().zip(hir_params) {
bx.set_span(hir_param.span);
self.declare_shader_interface_for_param(
execution_model,
entry_arg_abi,
hir_param,
&mut op_entry_point_interface_operands,
@ -284,8 +285,10 @@ impl<'tcx> CodegenCx<'tcx> {
(spirv_ty, storage_class)
}
#[allow(clippy::too_many_arguments)]
fn declare_shader_interface_for_param(
&self,
execution_model: ExecutionModel,
entry_arg_abi: &ArgAbi<'tcx, Ty<'tcx>>,
hir_param: &hir::Param<'tcx>,
op_entry_point_interface_operands: &mut Vec<Word>,
@ -522,11 +525,12 @@ impl<'tcx> CodegenCx<'tcx> {
}
self.check_for_bad_types(
execution_model,
hir_param.ty_span,
var_ptr_spirv_type,
storage_class,
attrs.builtin.is_some(),
attrs.flat.is_some(),
attrs.flat,
);
// Assign locations from left to right, incrementing each storage class
@ -568,45 +572,95 @@ impl<'tcx> CodegenCx<'tcx> {
}
// Booleans are only allowed in some storage classes. Error if they're in others.
// Integers and f64s must be decorated with `#[spirv(flat)]`.
// Integers and `f64`s must be decorated with `#[spirv(flat)]`.
fn check_for_bad_types(
&self,
execution_model: ExecutionModel,
span: Span,
ty: Word,
storage_class: StorageClass,
is_builtin: bool,
is_flat: bool,
flat_attr: Option<Spanned<()>>,
) {
// private and function are allowed here, but they can't happen.
if matches!(
storage_class,
StorageClass::Workgroup | StorageClass::CrossWorkgroup
) {
return;
}
let mut has_bool = false;
let mut type_must_be_flat = false;
recurse(self, ty, &mut has_bool, &mut type_must_be_flat);
// SPIR-V technically allows all input/output variables to be booleans, not just builtins,
// but has a note:
// > Khronos Issue #363: OpTypeBool can be used in the Input and Output storage classes,
// but the client APIs still only allow built-in Boolean variables (e.g. FrontFacing),
// not user variables.
// spirv-val disallows non-builtin inputs/outputs, so we do too, I guess.
if matches!(
storage_class,
StorageClass::Workgroup | StorageClass::CrossWorkgroup
) || is_builtin && matches!(storage_class, StorageClass::Input | StorageClass::Output)
{
return;
}
let mut has_bool = false;
let mut must_be_flat = false;
recurse(self, ty, &mut has_bool, &mut must_be_flat);
if has_bool {
self.tcx
.sess
.span_err(span, "entrypoint parameter cannot contain a boolean");
}
if matches!(storage_class, StorageClass::Input | StorageClass::Output)
&& must_be_flat
&& !is_flat
if has_bool
&& !(is_builtin && matches!(storage_class, StorageClass::Input | StorageClass::Output))
{
self.tcx
.sess
.span_err(span, "parameter must be decorated with #[spirv(flat)]");
.span_err(span, "entry-point parameter cannot contain `bool`s");
}
// Enforce Vulkan validation rules around `Flat` as accurately as possible,
// i.e. "interpolation control" can only be used "within" the rasterization
// pipeline (roughly: `vertex (outputs) -> ... -> (inputs for) fragment`),
// but not at the "outer" interface (vertex inputs/fragment outputs).
// Also, fragment inputs *require* it for some ("uninterpolatable") types.
// FIXME(eddyb) maybe this kind of `enum` could be placed elsewhere?
enum Force {
Disallow,
Require,
}
#[allow(clippy::match_same_arms)]
let flat_forced = match (execution_model, storage_class) {
// VUID-StandaloneSpirv-Flat-06202
// > The `Flat`, `NoPerspective`, `Sample`, and `Centroid` decorations **must**
// > not be used on variables with the `Input` storage class in a vertex shader
(ExecutionModel::Vertex, StorageClass::Input) => Some(Force::Disallow),
// VUID-StandaloneSpirv-Flat-04744
// > Any variable with integer or double-precision floating-point type and
// > with `Input` storage class in a fragment shader, **must** be decorated `Flat`
(ExecutionModel::Fragment, StorageClass::Input) if type_must_be_flat => {
// FIXME(eddyb) shouldn't this be automatic then? (maybe with a warning?)
Some(Force::Require)
}
// VUID-StandaloneSpirv-Flat-06201
// > The `Flat`, `NoPerspective`, `Sample`, and `Centroid` decorations **must**
// > not be used on variables with the `Output` storage class in a fragment shader
(ExecutionModel::Fragment, StorageClass::Output) => Some(Force::Disallow),
// VUID-StandaloneSpirv-Flat-04670
// > The `Flat`, `NoPerspective`, `Sample`, and `Centroid` decorations **must**
// > only be used on variables with the `Output` or `Input` storage class
(_, StorageClass::Input | StorageClass::Output) => None,
_ => Some(Force::Disallow),
};
let flat_mismatch = match (flat_forced, flat_attr) {
(Some(Force::Disallow), Some(flat_attr)) => Some((flat_attr.span, "cannot")),
// FIXME(eddyb) it would be useful to show the type that required it.
(Some(Force::Require), None) => Some((span, "must")),
_ => None,
};
if let Some((span, must_or_cannot)) = flat_mismatch {
self.tcx.sess.span_err(
span,
format!(
"`{execution_model:?}` entry-point `{storage_class:?}` parameter \
{must_or_cannot} be decorated with `#[spirv(flat)]`"
),
);
}
fn recurse(cx: &CodegenCx<'_>, ty: Word, has_bool: &mut bool, must_be_flat: &mut bool) {
match cx.lookup_type(ty) {
SpirvType::Bool => *has_bool = true,

View File

@ -6,7 +6,7 @@ use spirv_std::{glam::Vec4, ByteAddressableBuffer};
#[spirv(fragment)]
pub fn load(
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32],
#[spirv(flat)] out: &mut [i32; 4],
out: &mut [i32; 4],
) {
unsafe {
let buf = ByteAddressableBuffer::new(buf);

View File

@ -15,7 +15,7 @@ pub struct BigStruct {
#[spirv(fragment)]
pub fn load(
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32],
#[spirv(flat)] out: &mut BigStruct,
out: &mut BigStruct,
) {
unsafe {
let buf = ByteAddressableBuffer::new(buf);

View File

@ -8,7 +8,7 @@ pub struct EmptyStruct {}
#[spirv(fragment)]
pub fn load(
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32],
#[spirv(flat)] out: &mut EmptyStruct,
out: &mut EmptyStruct,
) {
unsafe {
let buf = ByteAddressableBuffer::new(buf);

View File

@ -11,7 +11,7 @@ pub struct SmallStruct {
#[spirv(fragment)]
pub fn load(
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32],
#[spirv(flat)] out: &mut SmallStruct,
out: &mut SmallStruct,
) {
unsafe {
let buf = ByteAddressableBuffer::new(buf);

View File

@ -6,7 +6,7 @@ use spirv_std::ByteAddressableBuffer;
#[spirv(fragment)]
pub fn load(
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32],
#[spirv(flat)] out: &mut u32,
out: &mut u32,
) {
unsafe {
let buf = ByteAddressableBuffer::new(buf);

View File

@ -24,7 +24,7 @@ impl Foo {
}
#[spirv(fragment)]
pub fn main(#[spirv(flat)] in_packed: u64, #[spirv(flat)] out_sum: &mut u32) {
pub fn main(#[spirv(flat)] in_packed: u64, out_sum: &mut u32) {
let foo = Foo::unpack(in_packed);
*out_sum = foo.a + (foo.b + foo.c) as u32;
}

View File

@ -2,7 +2,6 @@ use spirv_std::glam::Vec4;
use spirv_std::spirv;
use spirv_std::{image::Image2dArray, Sampler};
#[allow(unused_attributes)]
#[spirv(fragment)]
pub fn ps_main_stereo(
output: &mut Vec4,

View File

@ -7,7 +7,7 @@ use spirv_std::{arch, Image};
#[spirv(fragment)]
pub fn main(
#[spirv(descriptor_set = 0, binding = 0)] image: &Image!(2D, type=f32, sampled),
#[spirv(flat)] output: &mut u32,
output: &mut u32,
) {
*output = image.query_levels();
}

View File

@ -7,7 +7,7 @@ use spirv_std::{arch, Image};
#[spirv(fragment)]
pub fn main(
#[spirv(descriptor_set = 0, binding = 0)] image: &Image!(2D, type=f32, sampled, multisampled),
#[spirv(flat)] output: &mut u32,
output: &mut u32,
) {
*output = image.query_samples();
}

View File

@ -7,7 +7,7 @@ use spirv_std::{arch, Image};
#[spirv(fragment)]
pub fn main(
#[spirv(descriptor_set = 0, binding = 0)] image: &Image!(2D, type=f32, sampled=false),
#[spirv(flat)] output: &mut glam::UVec2,
output: &mut glam::UVec2,
) {
*output = image.query_size();
}

View File

@ -7,7 +7,7 @@ use spirv_std::{arch, Image};
#[spirv(fragment)]
pub fn main(
#[spirv(descriptor_set = 0, binding = 0)] image: &Image!(2D, type=f32, sampled),
#[spirv(flat)] output: &mut glam::UVec2,
output: &mut glam::UVec2,
) {
*output = image.query_size_lod(0);
}

View File

@ -8,8 +8,7 @@ const OFFSETS: [f32; 18] = [
33.368627,
];
#[allow(unused_attributes)]
#[spirv(fragment)]
pub fn main(#[spirv(flat)] x: &mut u32) {
pub fn main(x: &mut u32) {
*x = OFFSETS.len() as u32;
}

View File

@ -18,11 +18,11 @@ fn array3_deep_load(r: &'static [&'static u32; 3]) -> [u32; 3] {
}
#[spirv(fragment)]
pub fn main_pair(#[spirv(flat)] pair_out: &mut (u32, f32)) {
pub fn main_pair(pair_out: &mut (u32, f32)) {
*pair_out = pair_deep_load(&(&123, &3.14));
}
#[spirv(fragment)]
pub fn main_array3(#[spirv(flat)] array3_out: &mut [u32; 3]) {
pub fn main_array3(array3_out: &mut [u32; 3]) {
*array3_out = array3_deep_load(&[&0, &1, &2]);
}

View File

@ -22,9 +22,9 @@ fn deep_transpose(r: &'static &'static Mat2) -> Mat2 {
#[spirv(fragment)]
pub fn main(
#[spirv(flat)] scalar_out: &mut u32,
scalar_out: &mut u32,
#[spirv(push_constant)] vec_in: &Vec2,
#[spirv(flat)] bool_out: &mut u32,
bool_out: &mut u32,
vec_out: &mut Vec2,
) {
*scalar_out = deep_load(&&123);

View File

@ -15,12 +15,7 @@ fn scalar_load(r: &'static u32) -> u32 {
const ROT90: Mat2 = const_mat2![[0.0, 1.0], [-1.0, 0.0]];
#[spirv(fragment)]
pub fn main(
#[spirv(flat)] scalar_out: &mut u32,
vec_in: Vec2,
#[spirv(flat)] bool_out: &mut u32,
vec_out: &mut Vec2,
) {
pub fn main(scalar_out: &mut u32, vec_in: Vec2, bool_out: &mut u32, vec_out: &mut Vec2) {
*scalar_out = scalar_load(&123);
*bool_out = (vec_in == Vec2::ZERO) as u32;
*vec_out = ROT90.transpose() * vec_in;

View File

@ -10,7 +10,7 @@ fn closure_user<F: FnMut(&u32, u32)>(ptr: &u32, xmax: u32, mut callback: F) {
}
#[spirv(fragment)]
pub fn main(#[spirv(flat)] ptr: &mut u32) {
pub fn main(ptr: &mut u32) {
closure_user(ptr, 10, |ptr, i| {
if *ptr == i {
spirv_std::arch::kill();

View File

@ -11,6 +11,6 @@ fn has_two_decimal_digits(x: u32) -> bool {
}
#[spirv(fragment)]
pub fn main(#[spirv(flat)] i: u32, #[spirv(flat)] o: &mut u32) {
pub fn main(#[spirv(flat)] i: u32, o: &mut u32) {
*o = has_two_decimal_digits(i) as u32;
}

View File

@ -7,6 +7,6 @@
use spirv_std::spirv;
#[spirv(fragment)]
pub fn main(#[spirv(flat)] out: &mut u32) {
pub fn main(out: &mut u32) {
*out = None.unwrap_or(15);
}

View File

@ -6,7 +6,7 @@ use spirv_std::glam::{Vec2, Vec4};
use spirv_std::spirv;
#[spirv(fragment)]
pub fn test_vec2_to_f16x2(i: Vec2, #[spirv(flat)] o: &mut u32) {
pub fn test_vec2_to_f16x2(i: Vec2, o: &mut u32) {
*o = vec2_to_f16x2(i);
}
@ -16,7 +16,7 @@ pub fn test_f16x2_to_vec2(#[spirv(flat)] i: u32, o: &mut Vec2) {
}
#[spirv(fragment)]
pub fn test_f32_to_f16(i: f32, #[spirv(flat)] o: &mut u32) {
pub fn test_f32_to_f16(i: f32, o: &mut u32) {
*o = f32_to_f16(i);
}
@ -26,22 +26,22 @@ pub fn test_f16_to_f32(#[spirv(flat)] i: u32, o: &mut f32) {
}
#[spirv(fragment)]
pub fn test_vec4_to_u8x4_snorm(i: Vec4, #[spirv(flat)] o: &mut u32) {
pub fn test_vec4_to_u8x4_snorm(i: Vec4, o: &mut u32) {
*o = vec4_to_u8x4_snorm(i);
}
#[spirv(fragment)]
pub fn test_vec4_to_u8x4_unorm(i: Vec4, #[spirv(flat)] o: &mut u32) {
pub fn test_vec4_to_u8x4_unorm(i: Vec4, o: &mut u32) {
*o = vec4_to_u8x4_unorm(i);
}
#[spirv(fragment)]
pub fn test_vec2_to_u16x2_snorm(i: Vec2, #[spirv(flat)] o: &mut u32) {
pub fn test_vec2_to_u16x2_snorm(i: Vec2, o: &mut u32) {
*o = vec2_to_u16x2_snorm(i);
}
#[spirv(fragment)]
pub fn test_vec2_to_u16x2_unorm(i: Vec2, #[spirv(flat)] o: &mut u32) {
pub fn test_vec2_to_u16x2_unorm(i: Vec2, o: &mut u32) {
*o = vec2_to_u16x2_unorm(i);
}

View File

@ -29,7 +29,7 @@ struct CustomPair(u32, u32);
#[spirv(fragment)]
pub fn main(
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] slice: &[u32],
#[spirv(flat)] out: &mut u32,
out: &mut u32,
) {
let newtype_slice = Newtype(slice);
*out = newtype_slice.get()[0];

View File

@ -94,18 +94,18 @@ pub fn fragment(
#[spirv(clip_distance)] clip_distance: [f32; 1],
#[spirv(cull_distance)] cull_distance: [f32; 1],
#[spirv(frag_coord)] frag_coord: Vec4,
#[spirv(frag_invocation_count_ext)] frag_invocation_count_ext: u32,
#[spirv(frag_size_ext)] frag_size_ext: UVec2,
#[spirv(frag_invocation_count_ext, flat)] frag_invocation_count_ext: u32,
#[spirv(frag_size_ext, flat)] frag_size_ext: UVec2,
#[spirv(front_facing)] front_facing: bool,
#[spirv(fully_covered_ext)] fully_covered_ext: bool,
#[spirv(helper_invocation)] helper_invocation: bool,
#[spirv(layer)] layer: u32,
#[spirv(layer, flat)] layer: u32,
#[spirv(point_coord)] point_coord: Vec2,
#[spirv(primitive_id)] primitive_id: u32,
#[spirv(sample_id)] sample_id: u32,
#[spirv(sample_mask)] sample_mask: [u32; 1],
#[spirv(primitive_id, flat)] primitive_id: u32,
#[spirv(sample_id, flat)] sample_id: u32,
#[spirv(sample_mask, flat)] sample_mask: [u32; 1],
#[spirv(sample_position)] sample_position: Vec2,
#[spirv(viewport_index)] viewport_index: u32,
#[spirv(viewport_index, flat)] viewport_index: u32,
) {
}
#[spirv(closest_hit)]

View File

@ -1,22 +1,22 @@
error: entrypoint parameter cannot contain a boolean
error: entry-point parameter cannot contain `bool`s
--> $DIR/bool-inputs-err.rs:13:12
|
13 | input: bool,
| ^^^^
error: entrypoint parameter cannot contain a boolean
error: entry-point parameter cannot contain `bool`s
--> $DIR/bool-inputs-err.rs:14:13
|
14 | output: &mut bool,
| ^^^^^^^^^
error: entrypoint parameter cannot contain a boolean
error: entry-point parameter cannot contain `bool`s
--> $DIR/bool-inputs-err.rs:15:35
|
15 | #[spirv(push_constant)] push: &bool,
| ^^^^^
error: entrypoint parameter cannot contain a boolean
error: entry-point parameter cannot contain `bool`s
--> $DIR/bool-inputs-err.rs:16:32
|
16 | #[spirv(uniform)] uniform: &Boolthing,

View File

@ -1,10 +1,10 @@
error: parameter must be decorated with #[spirv(flat)]
error: `Fragment` entry-point `Input` parameter must be decorated with `#[spirv(flat)]`
--> $DIR/int-without-flat.rs:6:22
|
6 | pub fn fragment(int: u32, double: f64) {}
| ^^^
error: parameter must be decorated with #[spirv(flat)]
error: `Fragment` entry-point `Input` parameter must be decorated with `#[spirv(flat)]`
--> $DIR/int-without-flat.rs:6:35
|
6 | pub fn fragment(int: u32, double: f64) {}