anv: Flip around the way we reason about storage image lowering

There are roughly two cases when it comes to storage images.  In the
easy case, we have full hardware support and we can just emit a typed
read/write message in the shader and we're done.  In the more complex
cases, we may need to fall back to a typed read with a different format
or even to a raw (SSBO) read.

The hardware has always had basically full support for typed writes all
the way back to Ivy Bridge but typed reads have been harder to come by.
Starting with Skylake, we finally have enough that we at least have a
format of the right bit size but not necessarily the right format so we
can use a typed read but may still have to do an int->unorm or similar
cast in the shader.

Previously, in ANV, we treated lowered images as the default and write-
only as a special case that we can optimize.  This flips everything
around and treats the cases where we don't need to do any lowering as
the default "vanilla" case and treats the lowered case as special.
Importantly, this means that read-write access to surfaces where the
native format handles typed writes now use the same surface state as
write-only access and the only thing that uses the lowered surface state
is access read-write access with a format that doesn't support typed
reads.  This has the added benefit that now, if someone does a read
without specifying a format, we can default to the vanilla surface and
it will work as long as it's a format that supports typed reads.

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13198>
This commit is contained in:
Jason Ekstrand
2021-10-06 16:37:03 -05:00
parent fa251cf111
commit c0093c4668
5 changed files with 74 additions and 65 deletions

View File

@@ -1360,10 +1360,10 @@ anv_descriptor_set_write_image_view(struct anv_device *device,
assert(!(bind_layout->data & ANV_DESCRIPTOR_IMAGE_PARAM));
assert(image_view->n_planes == 1);
struct anv_storage_image_descriptor desc_data = {
.read_write = anv_surface_state_to_handle(
.vanilla = anv_surface_state_to_handle(
image_view->planes[0].storage_surface_state.state),
.write_only = anv_surface_state_to_handle(
image_view->planes[0].writeonly_storage_surface_state.state),
.lowered = anv_surface_state_to_handle(
image_view->planes[0].lowered_storage_surface_state.state),
};
memcpy(desc_map, &desc_data, sizeof(desc_data));
}
@@ -1372,7 +1372,7 @@ anv_descriptor_set_write_image_view(struct anv_device *device,
/* Storage images can only ever have one plane */
assert(image_view->n_planes == 1);
const struct brw_image_param *image_param =
&image_view->planes[0].storage_image_param;
&image_view->planes[0].lowered_storage_image_param;
anv_descriptor_set_write_image_param(desc_map, image_param);
}
@@ -1437,17 +1437,17 @@ anv_descriptor_set_write_buffer_view(struct anv_device *device,
if (bind_layout->data & ANV_DESCRIPTOR_STORAGE_IMAGE) {
assert(!(bind_layout->data & ANV_DESCRIPTOR_IMAGE_PARAM));
struct anv_storage_image_descriptor desc_data = {
.read_write = anv_surface_state_to_handle(
.vanilla = anv_surface_state_to_handle(
buffer_view->storage_surface_state),
.write_only = anv_surface_state_to_handle(
buffer_view->writeonly_storage_surface_state),
.lowered = anv_surface_state_to_handle(
buffer_view->lowered_storage_surface_state),
};
memcpy(desc_map, &desc_data, sizeof(desc_data));
}
if (bind_layout->data & ANV_DESCRIPTOR_IMAGE_PARAM) {
anv_descriptor_set_write_image_param(desc_map,
&buffer_view->storage_image_param);
&buffer_view->lowered_storage_image_param);
}
}

View File

@@ -2387,7 +2387,7 @@ anv_image_fill_surface_state(struct anv_device *device,
anv_image_address(image, &surface->memory_range);
if (view_usage == ISL_SURF_USAGE_STORAGE_BIT &&
!(flags & ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY) &&
(flags & ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED) &&
!isl_has_matching_typed_storage_image_format(&device->info,
view.format)) {
/* In this case, we are a writeable storage buffer which needs to be
@@ -2407,7 +2407,7 @@ anv_image_fill_surface_state(struct anv_device *device,
state_inout->clear_address = ANV_NULL_ADDRESS;
} else {
if (view_usage == ISL_SURF_USAGE_STORAGE_BIT &&
!(flags & ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY)) {
(flags & ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED)) {
/* Typed surface reads support a very limited subset of the shader
* image formats. Translate it into the closest format the hardware
* supports.
@@ -2628,37 +2628,38 @@ anv_CreateImageView(VkDevice _device,
/* NOTE: This one needs to go last since it may stomp isl_view.format */
if (iview->vk.usage & VK_IMAGE_USAGE_STORAGE_BIT) {
iview->planes[vplane].storage_surface_state.state = alloc_surface_state(device);
anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit,
&iview->planes[vplane].isl,
ISL_SURF_USAGE_STORAGE_BIT,
ISL_AUX_USAGE_NONE, NULL,
0,
&iview->planes[vplane].storage_surface_state,
NULL);
if (isl_is_storage_image_format(format.isl_format)) {
iview->planes[vplane].storage_surface_state.state =
iview->planes[vplane].lowered_storage_surface_state.state =
alloc_surface_state(device);
anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit,
&iview->planes[vplane].isl,
ISL_SURF_USAGE_STORAGE_BIT,
ISL_AUX_USAGE_NONE, NULL,
0,
&iview->planes[vplane].storage_surface_state,
&iview->planes[vplane].storage_image_param);
ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED,
&iview->planes[vplane].lowered_storage_surface_state,
&iview->planes[vplane].lowered_storage_image_param);
} else {
/* In this case, we support the format but, because there's no
* SPIR-V format specifier corresponding to it, we only support
* NonReadable (writeonly in GLSL) access. Instead of hanging in
* these invalid cases, we give them a NULL descriptor.
* SPIR-V format specifier corresponding to it, we only support it
* if the hardware can do it natively. This is possible for some
* reads but for most writes. Instead of hanging if someone gets
* it wrong, we give them a NULL descriptor.
*/
assert(isl_format_supports_typed_writes(&device->info,
format.isl_format));
iview->planes[vplane].storage_surface_state.state =
device->null_surface_state;
}
iview->planes[vplane].writeonly_storage_surface_state.state = alloc_surface_state(device);
anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit,
&iview->planes[vplane].isl,
ISL_SURF_USAGE_STORAGE_BIT,
ISL_AUX_USAGE_NONE, NULL,
ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY,
&iview->planes[vplane].writeonly_storage_surface_state,
NULL);
}
}
@@ -2697,9 +2698,9 @@ anv_DestroyImageView(VkDevice _device, VkImageView _iview,
iview->planes[plane].storage_surface_state.state);
}
if (iview->planes[plane].writeonly_storage_surface_state.state.offset) {
if (iview->planes[plane].lowered_storage_surface_state.state.offset) {
anv_state_pool_free(&device->surface_state_pool,
iview->planes[plane].writeonly_storage_surface_state.state);
iview->planes[plane].lowered_storage_surface_state.state);
}
}
@@ -2746,32 +2747,31 @@ anv_CreateBufferView(VkDevice _device,
if (buffer->usage & VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT) {
view->storage_surface_state = alloc_surface_state(device);
view->writeonly_storage_surface_state = alloc_surface_state(device);
view->lowered_storage_surface_state = alloc_surface_state(device);
enum isl_format storage_format =
anv_fill_buffer_surface_state(device, view->storage_surface_state,
view->format, ISL_SURF_USAGE_STORAGE_BIT,
view->address, view->range,
isl_format_get_layout(view->format)->bpb / 8);
enum isl_format lowered_format =
isl_has_matching_typed_storage_image_format(&device->info,
view->format) ?
isl_lower_storage_image_format(&device->info, view->format) :
ISL_FORMAT_RAW;
anv_fill_buffer_surface_state(device, view->storage_surface_state,
storage_format, ISL_SURF_USAGE_STORAGE_BIT,
anv_fill_buffer_surface_state(device, view->lowered_storage_surface_state,
lowered_format, ISL_SURF_USAGE_STORAGE_BIT,
view->address, view->range,
(storage_format == ISL_FORMAT_RAW ? 1 :
isl_format_get_layout(storage_format)->bpb / 8));
/* Write-only accesses should use the original format. */
anv_fill_buffer_surface_state(device, view->writeonly_storage_surface_state,
view->format, ISL_SURF_USAGE_STORAGE_BIT,
view->address, view->range,
isl_format_get_layout(view->format)->bpb / 8);
(lowered_format == ISL_FORMAT_RAW ? 1 :
isl_format_get_layout(lowered_format)->bpb / 8));
isl_buffer_fill_image_param(&device->isl_dev,
&view->storage_image_param,
&view->lowered_storage_image_param,
view->format, view->range);
} else {
view->storage_surface_state = (struct anv_state){ 0 };
view->writeonly_storage_surface_state = (struct anv_state){ 0 };
view->lowered_storage_surface_state = (struct anv_state){ 0 };
}
*pView = anv_buffer_view_to_handle(view);
@@ -2797,9 +2797,9 @@ anv_DestroyBufferView(VkDevice _device, VkBufferView bufferView,
anv_state_pool_free(&device->surface_state_pool,
view->storage_surface_state);
if (view->writeonly_storage_surface_state.alloc_size > 0)
if (view->lowered_storage_surface_state.alloc_size > 0)
anv_state_pool_free(&device->surface_state_pool,
view->writeonly_storage_surface_state);
view->lowered_storage_surface_state);
vk_object_free(&device->vk, pAllocator, view);
}

View File

@@ -1003,6 +1003,13 @@ lower_get_ssbo_size(nir_builder *b, nir_intrinsic_instr *intrin,
return true;
}
static bool
image_binding_needs_lowered_surface(nir_variable *var)
{
return !(var->data.access & ACCESS_NON_READABLE) &&
var->data.image.format != PIPE_FORMAT_NONE;
}
static bool
lower_image_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin,
struct apply_pipeline_layout_state *state)
@@ -1031,11 +1038,11 @@ lower_image_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin,
nir_ssa_def_rewrite_uses(&intrin->dest.ssa, desc);
} else if (binding_offset > MAX_BINDING_TABLE_SIZE) {
const bool write_only =
(var->data.access & ACCESS_NON_READABLE) != 0;
const unsigned desc_comp =
image_binding_needs_lowered_surface(var) ? 1 : 0;
nir_ssa_def *desc =
build_load_var_deref_descriptor_mem(b, deref, 0, 2, 32, state);
nir_ssa_def *handle = nir_channel(b, desc, write_only ? 1 : 0);
nir_ssa_def *handle = nir_channel(b, desc, desc_comp);
nir_rewrite_image_intrinsic(intrin, handle, true);
} else {
unsigned array_size =
@@ -1609,9 +1616,8 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice,
dim == GLSL_SAMPLER_DIM_SUBPASS_MS)
pipe_binding[i].input_attachment_index = var->data.index + i;
/* NOTE: This is a uint8_t so we really do need to != 0 here */
pipe_binding[i].write_only =
(var->data.access & ACCESS_NON_READABLE) != 0;
pipe_binding[i].lowered_storage_surface =
image_binding_needs_lowered_surface(var);
}
}

View File

@@ -1825,8 +1825,8 @@ struct anv_storage_image_descriptor {
* These are expected to already be shifted such that the 20-bit
* SURFACE_STATE table index is in the top 20 bits.
*/
uint32_t read_write;
uint32_t write_only;
uint32_t vanilla;
uint32_t lowered;
};
/** Struct representing a address/range descriptor
@@ -2027,9 +2027,9 @@ struct anv_buffer_view {
struct anv_state surface_state;
struct anv_state storage_surface_state;
struct anv_state writeonly_storage_surface_state;
struct anv_state lowered_storage_surface_state;
struct brw_image_param storage_image_param;
struct brw_image_param lowered_storage_image_param;
};
struct anv_push_descriptor_set {
@@ -2226,8 +2226,8 @@ struct anv_pipeline_binding {
uint8_t dynamic_offset_index;
};
/** For a storage image, whether it is write-only */
uint8_t write_only;
/** For a storage image, whether it requires a lowered surface */
uint8_t lowered_storage_surface;
/** Pad to 64 bits so that there are no holes and we can safely memcmp
* assuming POD zero-initialization.
@@ -4345,18 +4345,20 @@ struct anv_image_view {
/**
* RENDER_SURFACE_STATE when using image as a storage image. Separate
* states for write-only and readable, using the real format for
* write-only and the lowered format for readable.
* states for vanilla (with the original format) and one which has been
* lowered to a format suitable for reading. This may be a raw surface
* in extreme cases or simply a surface with a different format where we
* expect some conversion to be done in the shader.
*/
struct anv_surface_state storage_surface_state;
struct anv_surface_state writeonly_storage_surface_state;
struct anv_surface_state lowered_storage_surface_state;
struct brw_image_param storage_image_param;
struct brw_image_param lowered_storage_image_param;
} planes[3];
};
enum anv_image_view_state_flags {
ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY = (1 << 0),
ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED = (1 << 0),
ANV_IMAGE_VIEW_STATE_TEXTURE_OPTIMAL = (1 << 1),
};

View File

@@ -2762,8 +2762,9 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: {
if (desc->image_view) {
struct anv_surface_state sstate = (binding->write_only)
? desc->image_view->planes[binding->plane].writeonly_storage_surface_state
struct anv_surface_state sstate =
binding->lowered_storage_surface
? desc->image_view->planes[binding->plane].lowered_storage_surface_state
: desc->image_view->planes[binding->plane].storage_surface_state;
surface_state = sstate.state;
assert(surface_state.alloc_size);
@@ -2846,8 +2847,8 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
if (desc->buffer_view) {
surface_state = (binding->write_only)
? desc->buffer_view->writeonly_storage_surface_state
surface_state = binding->lowered_storage_surface
? desc->buffer_view->lowered_storage_surface_state
: desc->buffer_view->storage_surface_state;
assert(surface_state.alloc_size);
if (need_client_mem_relocs) {