[Mesa-dev] [PATCH 07/14] nir: add a pass to compact clip/cull distances.
Jason Ekstrand
jason at jlekstrand.net
Thu Nov 17 16:45:24 UTC 2016
On Wed, Nov 16, 2016 at 9:11 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:
> On Wednesday, November 16, 2016 8:37:50 PM PST Jason Ekstrand wrote:
> > On Mon, Nov 14, 2016 at 5:41 PM, Kenneth Graunke <kenneth at whitecape.org>
> > wrote:
> >
> > > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > > ---
> > > src/compiler/Makefile.sources | 1 +
> > > src/compiler/nir/nir.h | 1 +
> > > .../nir/nir_lower_clip_cull_distance_arrays.c | 187
> > > +++++++++++++++++++++
> > > 3 files changed, 189 insertions(+)
> > > create mode 100644 src/compiler/nir/nir_lower_
> clip_cull_distance_arrays.c
> > >
> > > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.
> sources
> > > index 08d93e0..6c331b0 100644
> > > --- a/src/compiler/Makefile.sources
> > > +++ b/src/compiler/Makefile.sources
> > > @@ -202,6 +202,7 @@ NIR_FILES = \
> > > nir/nir_lower_bitmap.c \
> > > nir/nir_lower_clamp_color_outputs.c \
> > > nir/nir_lower_clip.c \
> > > + nir/nir_lower_clip_cull_distance_arrays.c \
> > > nir/nir_lower_double_ops.c \
> > > nir/nir_lower_double_packing.c \
> > > nir/nir_lower_drawpixels.c \
> > > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > > index 0b78242..f3aadb9 100644
> > > --- a/src/compiler/nir/nir.h
> > > +++ b/src/compiler/nir/nir.h
> > > @@ -2441,6 +2441,7 @@ bool nir_lower_idiv(nir_shader *shader);
> > >
> > > void nir_lower_clip_vs(nir_shader *shader, unsigned ucp_enables);
> > > void nir_lower_clip_fs(nir_shader *shader, unsigned ucp_enables);
> > > +void nir_lower_clip_cull_distance_arrays(nir_shader *nir);
> > >
> > > void nir_lower_two_sided_color(nir_shader *shader);
> > >
> > > diff --git a/src/compiler/nir/nir_lower_clip_cull_distance_arrays.c
> > > b/src/compiler/nir/nir_lower_clip_cull_distance_arrays.c
> > > new file mode 100644
> > > index 0000000..9f8a7de
> > > --- /dev/null
> > > +++ b/src/compiler/nir/nir_lower_clip_cull_distance_arrays.c
> > > @@ -0,0 +1,187 @@
> > > +/*
> > > + * Copyright © 2015 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person
> obtaining a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom
> the
> > > + * Software is furnished to do so, subject to the following
> conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including
> the
> > > next
> > > + * paragraph) shall be included in all copies or substantial portions
> of
> > > the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR
> > > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > DEALINGS
> > > + * IN THE SOFTWARE.
> > > + */
> > > +
> > > +#include "nir.h"
> > > +#include "nir_builder.h"
> > > +
> > > +/**
> > > + * @file
> > > + *
> > > + * This pass combines separate clip and cull distance arrays into a
> > > + * single array that contains both. Clip distances come first, then
> > > + * cull distances. It also populates nir_shader_info with the size
> > > + * of the original arrays so the driver knows which are which.
> > > + */
> > > +
> > > +/**
> > > + * Get the length of the clip/cull distance array, looking past
> > > + * any interface block arrays.
> > > + */
> > > +static unsigned
> > > +get_unwrapped_array_length(nir_variable *var)
> > > +{
> > > + if (!var)
> > > + return 0;
> > > +
> > > + /* Unwrap GS input and TCS input/output interfaces. We want the
> > > + * underlying clip/cull distance array length, not the per-vertex
> > > + * array length.
> > > + */
> > >
> >
> > Probably not needed, but we could put some stage-based asserts in here.
>
> Yeah...we should probably put competent helpers for "is this
> per-vertex IO?" and "get me the actual underlying type please"
> in nir.h where everybody can use them.
>
That would be nice. At least the nir_var_is_per_vertex_io. The "get me
the correct type please" is just
if (is_per_vertex_io(var))
return glsl_get_array_element(var->type);
else
return var->type.
Actually, I think we may even be able to use var->interface_type in most of
those cases.
> > > + const struct glsl_type *type = var->type;
> > > + if (glsl_type_is_array_of_arrays(type))
> > > + type = glsl_get_array_element(type);
> > > +
> > > + return glsl_get_length(type);
> > > +}
> > > +
> > > +/**
> > > + * Get the type of the combined array (including interface block
> nesting).
> > > + */
> > > +static const struct glsl_type *
> > > +get_new_array_type(const struct glsl_type *old_type, unsigned length)
> > > +{
> > > + const struct glsl_type *type = glsl_array_type(glsl_float_type(),
> > > length);
> > > +
> > > + if (glsl_type_is_array_of_arrays(old_type))
> > > + type = glsl_array_type(type, glsl_get_length(type));
> > > +
> > > + return type;
> > > +}
> > > +
> > > +/**
> > > + * Rewrite any clip/cull distances to refer to the new combined array.
> > > + */
> > > +static void
> > > +rewrite_references(nir_instr *instr,
> > > + nir_variable *combined,
> > > + unsigned cull_offset)
> > > +{
> > > + if (instr->type != nir_instr_type_intrinsic)
> > > + return;
> > > +
> > > + nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> > > +
> > > + /* copy_var needs to be lowered to load/store before calling this
> pass
> > > */
> > > + assert(intrin->intrinsic != nir_intrinsic_copy_var);
> > >
> >
> > Wouldn't be that hard to make it support it, but sure.
> >
> >
> > > +
> > > + if (intrin->intrinsic != nir_intrinsic_load_var &&
> > > + intrin->intrinsic != nir_intrinsic_store_var)
> > > + return;
> > > +
> > > + nir_deref_var *var_ref = intrin->variables[0];
> > > + if (var_ref->var->data.mode != combined->data.mode)
> > > + return;
> > > +
> > > + if (var_ref->var->data.location != VARYING_SLOT_CLIP_DIST0 &&
> > > + var_ref->var->data.location != VARYING_SLOT_CULL_DIST0)
> > > + return;
> > > +
> > > + /* Update types along the deref chain */
> > > + const struct glsl_type *type = combined->type;
> > > + nir_deref *deref = &var_ref->deref;
> > > + while (deref) {
> > > + deref->type = type;
> > > + deref = deref->child;
> > > + type = glsl_get_array_element(type);
> > > + }
> > > +
> > > + /* For cull distances, add an offset to the array index */
> > > + if (var_ref->var->data.location == VARYING_SLOT_CULL_DIST0) {
> > > + nir_deref *tail = nir_deref_tail(&intrin->variables[0]->deref);
> > > + nir_deref_array *array_ref = nir_deref_as_array(tail);
> > > +
> > > + array_ref->base_offset += cull_offset;
> > > + }
> > > +
> > > + /* Point the deref at the combined array */
> > > + var_ref->var = combined;
> > > +
> > > + /* There's no need to update writemasks; it's a scalar array. */
> > > +}
> > > +
> > > +static void
> > > +combine_clip_cull(nir_shader *nir,
> > > + struct exec_list *vars,
> > > + bool store_info)
> > > +{
> > > + nir_variable *cull = NULL;
> > > + nir_variable *clip = NULL;
> > > +
> > > + nir_foreach_variable(var, vars) {
> > > + if (var->data.location == VARYING_SLOT_CLIP_DIST0)
> > > + clip = var;
> > > +
> > > + if (var->data.location == VARYING_SLOT_CULL_DIST0)
> > > + cull = var;
> > > + }
> > > +
> > > + const unsigned clip_array_size = get_unwrapped_array_length(clip);
> > > + const unsigned cull_array_size = get_unwrapped_array_length(cull);
> > > +
> > > + if (store_info) {
> > > + nir->info->clip_distance_array_size = clip_array_size;
> > > + nir->info->cull_distance_array_size = cull_array_size;
> > >
> >
> > Why is this optional? If the sizes here and in the info ever differ,
> > you're going to have a problem.
>
> We call this function twice - once for inputs, and once for outputs.
> The info values should come from the shader outputs. Though, looking
> below......
>
Right... I forgot about in vs. out. Maybe we could pick a better name like
num_clip_out and num_cull_out?
I made a bunch of comments. I'm not extraordinarily set on any of them so
do with it what you will. 1-11 and 13-14 are
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> > > + }
> > > +
> > > + if (clip)
> > > + clip->data.compact = true;
> > > +
> > > + if (cull)
> > > + cull->data.compact = true;
> > > +
> > > + if (cull_array_size > 0) {
> > > + if (clip_array_size == 0) {
> > > + /* No clip distances, just change the cull distance location
> */
> > > + cull->data.location = VARYING_SLOT_CLIP_DIST0;
> > > + } else {
> > > + /* Turn the ClipDistance array into a combined one */
> > > + clip->type = get_new_array_type(clip->type,
> > > + clip_array_size +
> > > cull_array_size);
> > > +
> > > + /* Rewrite CullDistance to reference the combined array */
> > > + nir_foreach_function(function, nir) {
> > > + if (function->impl) {
> > > + nir_foreach_block(block, function->impl) {
> > > + nir_foreach_instr(instr, block) {
> > > + rewrite_references(instr, clip, clip_array_size);
> > > + }
> > > + }
> > > + }
> > > + }
> > > +
> > > + /* Delete the old CullDistance variable */
> > > + exec_node_remove(&cull->node);
> > > + ralloc_free(cull);
> > > + }
> > > + }
> > > +}
> > > +
> > > +void
> > > +nir_lower_clip_cull_distance_arrays(nir_shader *nir)
> > > +{
> > > + if (nir->stage <= MESA_SHADER_GEOMETRY)
> > > + combine_clip_cull(nir, &nir->outputs, &nir->info);
> > > +
> > > + if (nir->stage > MESA_SHADER_VERTEX)
> > > + combine_clip_cull(nir, &nir->inputs, NULL);
> > > +}
>
> .....apparently I meant to pass "true" and "false" here.
>
> (I'd originally passed the shader_info pointer, rather than a flag,
> but once I started passing nir_shader as well, I changed it. Oops.)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161117/3668ef9a/attachment-0001.html>
More information about the mesa-dev
mailing list