Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[spv-out] Fix non-uniformity attributes #2421

Closed
wants to merge 1 commit into from
Closed

[spv-out] Fix non-uniformity attributes #2421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
136 changes: 102 additions & 34 deletions src/back/spv/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn get_dimension(type_inner: &crate::TypeInner) -> Dimension {
enum ExpressionPointer {
/// The pointer to the expression's value is available, as the value of the
/// expression with the given id.
Ready { pointer_id: Word },
Ready { pointer_id: Word, non_uniform: bool },

/// The access expression must be conditional on the value of `condition`, a boolean
/// expression that is true if all indices are in bounds. If `condition` is true, then
Expand All @@ -35,6 +35,7 @@ enum ExpressionPointer {
Conditional {
condition: Word,
access: Instruction,
non_uniform: bool,
},
}

Expand Down Expand Up @@ -285,12 +286,15 @@ impl<'w> BlockContext<'w> {
class: helpers::map_storage_class(space),
});

let result_id = match self.write_expression_pointer(
let (result_id, non_uniform) = match self.write_expression_pointer(
expr_handle,
block,
Some(binding_array_false_pointer),
)? {
ExpressionPointer::Ready { pointer_id } => pointer_id,
ExpressionPointer::Ready {
pointer_id,
non_uniform,
} => (pointer_id, non_uniform),
ExpressionPointer::Conditional { .. } => {
return Err(Error::FeatureNotImplemented(
"Texture array out-of-bounds handling",
Expand All @@ -308,6 +312,11 @@ impl<'w> BlockContext<'w> {
None,
));

if non_uniform {
self.writer
.decorate_non_uniform_binding_array_access(load_id)?;
}

load_id
}
ref other => {
Expand Down Expand Up @@ -361,12 +370,15 @@ impl<'w> BlockContext<'w> {
class: helpers::map_storage_class(space),
});

let result_id = match self.write_expression_pointer(
let (result_id, non_uniform) = match self.write_expression_pointer(
expr_handle,
block,
Some(binding_array_false_pointer),
)? {
ExpressionPointer::Ready { pointer_id } => pointer_id,
ExpressionPointer::Ready {
pointer_id,
non_uniform,
} => (pointer_id, non_uniform),
ExpressionPointer::Conditional { .. } => {
return Err(Error::FeatureNotImplemented(
"Texture array out-of-bounds handling",
Expand All @@ -384,6 +396,11 @@ impl<'w> BlockContext<'w> {
None,
));

if non_uniform {
self.writer
.decorate_non_uniform_binding_array_access(load_id)?;
}

load_id
}
ref other => {
Expand Down Expand Up @@ -1052,7 +1069,10 @@ impl<'w> BlockContext<'w> {
crate::Expression::LocalVariable(variable) => self.function.variables[&variable].id,
crate::Expression::Load { pointer } => {
match self.write_expression_pointer(pointer, block, None)? {
ExpressionPointer::Ready { pointer_id } => {
ExpressionPointer::Ready {
pointer_id,
non_uniform,
} => {
let id = self.gen_id();
let atomic_space =
match *self.fun_info[pointer].ty.inner_with(&self.ir_module.types) {
Expand All @@ -1079,15 +1099,23 @@ impl<'w> BlockContext<'w> {
Instruction::load(result_type_id, id, pointer_id, None)
};
block.body.push(instruction);
if non_uniform {
self.writer.decorate_non_uniform_binding_array_access(id)?;
}
id
}
ExpressionPointer::Conditional { condition, access } => {
ExpressionPointer::Conditional {
condition,
access,
non_uniform,
} => {
//TODO: support atomics?
self.write_conditional_indexed_load(
let mut non_uniform_instruction = None;
let res = self.write_conditional_indexed_load(
result_type_id,
condition,
block,
move |id_gen, block| {
|id_gen, block| {
// The in-bounds path. Perform the access and the load.
let pointer_id = access.result_id.unwrap();
let value_id = id_gen.next();
Expand All @@ -1098,9 +1126,17 @@ impl<'w> BlockContext<'w> {
pointer_id,
None,
));
value_id
if non_uniform {
non_uniform_instruction = Some(value_id);
}
Ok(value_id)
},
)
)?;
if let Some(value_id) = non_uniform_instruction {
self.writer
.decorate_non_uniform_binding_array_access(value_id)?;
}
res
}
}
}
Expand Down Expand Up @@ -1517,13 +1553,11 @@ impl<'w> BlockContext<'w> {
}
};

let (pointer_id, expr_pointer) = if self.temp_list.is_empty() {
(
root_id,
ExpressionPointer::Ready {
pointer_id: root_id,
},
)
let expr_pointer = if self.temp_list.is_empty() {
ExpressionPointer::Ready {
pointer_id: root_id,
non_uniform: is_non_uniform_binding_array,
}
} else {
self.temp_list.reverse();
let pointer_id = self.gen_id();
Expand All @@ -1534,19 +1568,21 @@ impl<'w> BlockContext<'w> {
// caller to generate the branch, the access, the load or store, and
// the zero value (for loads). Otherwise, we can emit the access
// ourselves, and just hand them the id of the pointer.
let expr_pointer = match accumulated_checks {
Some(condition) => ExpressionPointer::Conditional { condition, access },
match accumulated_checks {
Some(condition) => ExpressionPointer::Conditional {
condition,
access,
non_uniform: is_non_uniform_binding_array,
},
None => {
block.body.push(access);
ExpressionPointer::Ready { pointer_id }
ExpressionPointer::Ready {
pointer_id,
non_uniform: is_non_uniform_binding_array,
}
}
};
(pointer_id, expr_pointer)
}
};
if is_non_uniform_binding_array {
self.writer
.decorate_non_uniform_binding_array_access(pointer_id)?;
}

Ok(expr_pointer)
}
Expand Down Expand Up @@ -2003,7 +2039,10 @@ impl<'w> BlockContext<'w> {
crate::Statement::Store { pointer, value } => {
let value_id = self.cached[value];
match self.write_expression_pointer(pointer, &mut block, None)? {
ExpressionPointer::Ready { pointer_id } => {
ExpressionPointer::Ready {
pointer_id,
non_uniform,
} => {
let atomic_space = match *self.fun_info[pointer]
.ty
.inner_with(&self.ir_module.types)
Expand All @@ -2029,9 +2068,17 @@ impl<'w> BlockContext<'w> {
} else {
Instruction::store(pointer_id, value_id, None)
};
if non_uniform {
self.writer
.decorate_non_uniform_binding_array_access(pointer_id)?;
}
block.body.push(instruction);
}
ExpressionPointer::Conditional { condition, access } => {
ExpressionPointer::Conditional {
condition,
access,
non_uniform,
} => {
let mut selection = Selection::start(&mut block, ());
selection.if_true(self, condition, ());

Expand All @@ -2042,6 +2089,10 @@ impl<'w> BlockContext<'w> {
.block()
.body
.push(Instruction::store(pointer_id, value_id, None));
if non_uniform {
self.writer
.decorate_non_uniform_binding_array_access(pointer_id)?;
}

// Finish the in-bounds block and start the merge block. This
// is the block we'll leave current on return.
Expand Down Expand Up @@ -2094,7 +2145,16 @@ impl<'w> BlockContext<'w> {

let pointer_id =
match self.write_expression_pointer(pointer, &mut block, None)? {
ExpressionPointer::Ready { pointer_id } => pointer_id,
ExpressionPointer::Ready {
pointer_id,
non_uniform,
} => {
if non_uniform {
self.writer
.decorate_non_uniform_binding_array_access(pointer_id)?;
}
pointer_id
}
ExpressionPointer::Conditional { .. } => {
return Err(Error::FeatureNotImplemented(
"Atomics out-of-bounds handling",
Expand Down Expand Up @@ -2269,7 +2329,11 @@ impl<'w> BlockContext<'w> {
let result_type_id = self.get_expression_type_id(&self.fun_info[result].ty);
// Embed the body of
match self.write_expression_pointer(pointer, &mut block, None)? {
ExpressionPointer::Ready { pointer_id } => {
// workgroup uniform loads can't ever be non-uniform
ExpressionPointer::Ready {
pointer_id,
non_uniform: _,
} => {
let id = self.gen_id();
block.body.push(Instruction::load(
result_type_id,
Expand All @@ -2279,7 +2343,11 @@ impl<'w> BlockContext<'w> {
));
self.cached[result] = id;
}
ExpressionPointer::Conditional { condition, access } => {
ExpressionPointer::Conditional {
condition,
access,
non_uniform: _,
} => {
self.cached[result] = self.write_conditional_indexed_load(
result_type_id,
condition,
Expand All @@ -2295,9 +2363,9 @@ impl<'w> BlockContext<'w> {
pointer_id,
None,
));
value_id
Ok(value_id)
},
)
)?
}
}
self.writer
Expand Down
12 changes: 6 additions & 6 deletions src/back/spv/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,9 @@ impl<'w> BlockContext<'w> {
condition: Word,
block: &mut Block,
emit_load: F,
) -> Word
) -> Result<Word, Error>
where
F: FnOnce(&mut IdGenerator, &mut Block) -> Word,
F: FnOnce(&mut IdGenerator, &mut Block) -> Result<Word, Error>,
{
// For the out-of-bounds case, we produce a zero value.
let null_id = self.writer.get_constant_null(result_type);
Expand All @@ -324,9 +324,9 @@ impl<'w> BlockContext<'w> {
selection.if_true(self, condition, null_id);

// The in-bounds path. Perform the access and the load.
let loaded_value = emit_load(&mut self.writer.id_gen, selection.block());
let loaded_value = emit_load(&mut self.writer.id_gen, selection.block())?;

selection.finish(self, loaded_value)
Ok(selection.finish(self, loaded_value))
}

/// Emit code for bounds checks for an array, vector, or matrix access.
Expand Down Expand Up @@ -410,9 +410,9 @@ impl<'w> BlockContext<'w> {
base_id,
index_id,
));
element_id
Ok(element_id)
},
)
)?
}
};

Expand Down
44 changes: 22 additions & 22 deletions tests/out/spv/binding-arrays.spvasm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,28 @@ OpMemberDecorate %47 0 Offset 0
OpDecorate %63 Location 0
OpDecorate %63 Flat
OpDecorate %66 Location 0
OpDecorate %95 NonUniform
OpDecorate %118 NonUniform
OpDecorate %120 NonUniform
OpDecorate %145 NonUniform
OpDecorate %147 NonUniform
OpDecorate %183 NonUniform
OpDecorate %211 NonUniform
OpDecorate %227 NonUniform
OpDecorate %243 NonUniform
OpDecorate %264 NonUniform
OpDecorate %266 NonUniform
OpDecorate %288 NonUniform
OpDecorate %290 NonUniform
OpDecorate %312 NonUniform
OpDecorate %314 NonUniform
OpDecorate %336 NonUniform
OpDecorate %338 NonUniform
OpDecorate %360 NonUniform
OpDecorate %362 NonUniform
OpDecorate %384 NonUniform
OpDecorate %386 NonUniform
OpDecorate %409 NonUniform
OpDecorate %96 NonUniform
OpDecorate %119 NonUniform
OpDecorate %121 NonUniform
OpDecorate %146 NonUniform
OpDecorate %148 NonUniform
OpDecorate %184 NonUniform
OpDecorate %212 NonUniform
OpDecorate %228 NonUniform
OpDecorate %244 NonUniform
OpDecorate %265 NonUniform
OpDecorate %267 NonUniform
OpDecorate %289 NonUniform
OpDecorate %291 NonUniform
OpDecorate %313 NonUniform
OpDecorate %315 NonUniform
OpDecorate %337 NonUniform
OpDecorate %339 NonUniform
OpDecorate %361 NonUniform
OpDecorate %363 NonUniform
OpDecorate %385 NonUniform
OpDecorate %387 NonUniform
OpDecorate %410 NonUniform
%2 = OpTypeVoid
%3 = OpTypeInt 32 0
%4 = OpTypeStruct %3
Expand Down
2 changes: 1 addition & 1 deletion tests/out/spv/binding-buffer-arrays.spvasm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ OpMemberDecorate %16 0 Offset 0
OpDecorate %23 Location 0
OpDecorate %23 Flat
OpDecorate %26 Location 0
OpDecorate %55 NonUniform
OpDecorate %58 NonUniform
Copy link
Member

Choose a reason for hiding this comment

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

I think for buffers we need to keep the decoration on the access chain.

%2 = OpTypeVoid
%3 = OpTypeInt 32 0
%4 = OpTypeStruct %3
Expand Down