[Mesa-dev] [PATCH 4/6] nir: update nir_gather_info to only mark used array/matrix elements
Kenneth Graunke
kenneth at whitecape.org
Thu Nov 10 10:46:35 UTC 2016
On Thursday, October 27, 2016 1:00:46 PM PST Timothy Arceri wrote:
> This is based on the code from the GLSL IR pass however unlike the GLSL IR
> pass it also supports arrays of arrays.
>
> As well as implementing the logic from the GLSL IR pass we add some
> additional intrinsic cases to catch more system values.
> ---
> src/compiler/nir/nir_gather_info.c | 262 +++++++++++++++++++++++++++++--------
> 1 file changed, 209 insertions(+), 53 deletions(-)
This looks really nice. Patches 2 and 4 are
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
One question and a couple of small suggestions below...
>
> diff --git a/src/compiler/nir/nir_gather_info.c b/src/compiler/nir/nir_gather_info.c
> index 380140a..e5cedd9 100644
> --- a/src/compiler/nir/nir_gather_info.c
> +++ b/src/compiler/nir/nir_gather_info.c
> @@ -21,9 +21,191 @@
> * IN THE SOFTWARE.
> */
>
> +#include "main/mtypes.h"
> #include "nir.h"
>
> static void
> +set_io_mask(nir_shader *shader, nir_variable *var, int offset, int len)
> +{
> + for (int i = 0; i < len; i++) {
> + assert(var->data.location != -1);
> +
> + int idx = var->data.location + offset + i;
> + bool is_patch_generic = var->data.patch &&
> + idx != VARYING_SLOT_TESS_LEVEL_INNER &&
> + idx != VARYING_SLOT_TESS_LEVEL_OUTER &&
> + idx != VARYING_SLOT_BOUNDING_BOX0 &&
> + idx != VARYING_SLOT_BOUNDING_BOX1;
> + uint64_t bitfield;
> +
> + if (is_patch_generic) {
> + assert(idx >= VARYING_SLOT_PATCH0 && idx < VARYING_SLOT_TESS_MAX);
> + bitfield = BITFIELD64_BIT(idx - VARYING_SLOT_PATCH0);
> + }
> + else {
> + assert(idx < VARYING_SLOT_MAX);
> + bitfield = BITFIELD64_BIT(idx);
> + }
> +
> + if (var->data.mode == nir_var_shader_in) {
> + if (is_patch_generic)
> + shader->info->patch_inputs_read |= bitfield;
> + else
> + shader->info->inputs_read |= bitfield;
> +
> + /* double inputs read is only for vertex inputs */
> + if (shader->stage == MESA_SHADER_VERTEX &&
> + glsl_type_is_dual_slot(glsl_without_array(var->type)))
> + shader->info->double_inputs_read |= bitfield;
> +
> + if (shader->stage == MESA_SHADER_FRAGMENT) {
> + shader->info->fs.uses_sample_qualifier |= var->data.sample;
> + }
> + } else {
> + assert(var->data.mode == nir_var_shader_out);
> + if (is_patch_generic) {
> + shader->info->patch_outputs_written |= bitfield;
> + } else if (!var->data.read_only) {
> + shader->info->outputs_written |= bitfield;
> + }
> +
> + if (var->data.fb_fetch_output)
> + shader->info->outputs_read |= bitfield;
> + }
> + }
> +}
> +
> +/**
> + * Mark an entire variable as used. Caller must ensure that the variable
> + * represents a shader input or output.
> + */
> +static void
> +mark_whole_variable(nir_shader *shader, nir_variable *var)
> +{
> + const struct glsl_type *type = var->type;
> + bool is_vertex_input = false;
> +
> + if (nir_is_per_vertex_io(var, shader->stage)) {
> + assert(glsl_type_is_array(type));
> + type = glsl_get_array_element(type);
> + }
> +
> + if (shader->stage == MESA_SHADER_VERTEX &&
> + var->data.mode == nir_var_shader_in)
> + is_vertex_input = true;
> +
> + set_io_mask(shader, var, 0,
> + glsl_count_attribute_slots(type, is_vertex_input));
> +}
> +
> +static unsigned
> +get_io_offset(nir_deref_var *deref, bool is_vertex_input)
> +{
> + unsigned offset = 0;
> +
> + nir_deref *tail = &deref->deref;
> + while (tail->child != NULL) {
> + tail = tail->child;
> +
> + if (tail->deref_type == nir_deref_type_array) {
> + nir_deref_array *deref_array = nir_deref_as_array(tail);
> +
> + if (deref_array->deref_array_type == nir_deref_array_type_indirect) {
> + return -1;
> + }
> +
> + offset += glsl_count_attribute_slots(tail->type, is_vertex_input) *
> + deref_array->base_offset;
> + }
Do we need any dual slot handling when computing the offset?
Or is that OK?
> + /* TODO: we can get the offset for structs here see nir_lower_io() */
> + }
> +
> + return offset;
> +}
> +
> +/**
> + * Try to mark a portion of the given varying as used. Caller must ensure
> + * that the variable represents a shader input or output.
> + *
> + * If the index can't be interpreted as a constant, or some other problem
> + * occurs, then nothing will be marked and false will be returned.
> + */
> +static bool
> +try_mask_partial_io(nir_shader *shader, nir_deref_var *deref)
> +{
> + nir_variable *var = deref->var;
> + const struct glsl_type *type = var->type;
> +
> + if (nir_is_per_vertex_io(var, shader->stage)) {
> + assert(glsl_type_is_array(type));
> + type = glsl_get_array_element(type);
> + }
> +
> + /* The code below only handles:
> + *
> + * - Indexing into matrices
> + * - Indexing into arrays of (arrays, matrices, vectors, or scalars)
> + *
> + * For now, we just give up if we see varying structs and arrays of structs
> + * here marking the entire variable as used.
> + */
> + if (!(glsl_type_is_matrix(type) ||
> + (glsl_type_is_array(type) &&
> + (glsl_type_is_numeric(glsl_without_array(type)) ||
> + glsl_type_is_boolean(glsl_without_array(type)))))) {
The last three lines should be indented one space further.
(Looks like the bad indentation was copied from ir_set_program_inouts.cpp.)
> +
> + /* If we don't know how to handle this case, give up and let the
> + * caller mark the whole variable as used.
> + */
> + return false;
> + }
> +
> + bool is_vertex_input = false;
> + if (shader->stage == MESA_SHADER_VERTEX &&
> + var->data.mode == nir_var_shader_in)
> + is_vertex_input = true;
> +
> + unsigned offset = get_io_offset(deref, is_vertex_input);
> + if (offset == -1)
> + return false;
> +
> + unsigned num_elems;
> + unsigned elem_width = 1;
> + unsigned mat_cols = 1;
> + if (glsl_type_is_array(type)) {
> + num_elems = glsl_get_aoa_size(type);
> + if (glsl_type_is_matrix(glsl_without_array(type)))
> + mat_cols = glsl_get_matrix_columns(glsl_without_array(type));
> + } else {
> + num_elems = glsl_get_matrix_columns(type);
> + }
> +
> + /* double element width for double types that takes two slots */
> + if (shader->stage != MESA_SHADER_VERTEX ||
> + var->data.mode != nir_var_shader_in) {
> + if (glsl_type_is_dual_slot(glsl_without_array(type))) {
> + elem_width *= 2;
> + }
> + }
You've got a boolean for this, so you can simplify to:
if (!is_vertex_input &&
glsl_type_is_dual_slot(glsl_without_array(type))) {
elem_width *= 2;
}
(looks like ir_set_program_inouts didn't have that boolean)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161110/a0e53f22/attachment.sig>
More information about the mesa-dev
mailing list