Acked-by: Marek Olšák <marek.ol...@amd.com> Marek
On Fri, Aug 31, 2018 at 11:11 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > We were going out of our way to disable dual-location re-mapping in NIR > only to then do the remapping in st_glsl_to_nir.cpp. Presumably, this > was so that double_inputs would be correct for the core state tracker. > However, now that we've it to gl_program::DualSlotInputs which is > unaffected by NIR lowering, we can let NIR lower things for us. The one > tricky bit here is that we have to remap the inputs_read bitfield back > to the single-slot convention for the gallium state tracker to use. > > Since radeonsi is the only NIR-capable gallium driver that also supports > GL_ARB_vertex_attrib_64bit, we only have to worry about radeonsi when > making core gallium state tracker changes. > --- > .../nir/nir_lower_io_arrays_to_elements.c | 5 +-- > src/gallium/drivers/radeonsi/si_get.c | 1 + > src/mesa/state_tracker/st_glsl_to_nir.cpp | 45 +++++++++---------- > 3 files changed, 22 insertions(+), 29 deletions(-) > > diff --git a/src/compiler/nir/nir_lower_io_arrays_to_elements.c > b/src/compiler/nir/nir_lower_io_arrays_to_elements.c > index 16f6233f614..af33d153ea5 100644 > --- a/src/compiler/nir/nir_lower_io_arrays_to_elements.c > +++ b/src/compiler/nir/nir_lower_io_arrays_to_elements.c > @@ -36,9 +36,6 @@ static unsigned > get_io_offset(nir_builder *b, nir_deref_instr *deref, nir_variable *var, > unsigned *element_index, nir_ssa_def **vertex_index) > { > - bool vs_in = (b->shader->info.stage == MESA_SHADER_VERTEX) && > - (var->data.mode == nir_var_shader_in); > - > nir_deref_path path; > nir_deref_path_init(&path, deref, NULL); > > @@ -60,7 +57,7 @@ get_io_offset(nir_builder *b, nir_deref_instr *deref, > nir_variable *var, > > assert(c); /* must not be indirect dereference */ > > - unsigned size = glsl_count_attribute_slots((*p)->type, vs_in); > + unsigned size = glsl_count_attribute_slots((*p)->type, false); > offset += size * c->u32[0]; > > unsigned num_elements = glsl_type_is_array((*p)->type) ? > diff --git a/src/gallium/drivers/radeonsi/si_get.c > b/src/gallium/drivers/radeonsi/si_get.c > index 90f62edf470..bc3d559861f 100644 > --- a/src/gallium/drivers/radeonsi/si_get.c > +++ b/src/gallium/drivers/radeonsi/si_get.c > @@ -497,6 +497,7 @@ static const struct nir_shader_compiler_options > nir_options = { > .lower_extract_word = true, > .max_unroll_iterations = 32, > .native_integers = true, > + .vs_inputs_dual_locations = true, > }; > > static const void * > diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp > b/src/mesa/state_tracker/st_glsl_to_nir.cpp > index 0ee9bd9fef1..d0ec410ec69 100644 > --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp > @@ -32,6 +32,7 @@ > #include "program/prog_parameter.h" > #include "program/ir_to_mesa.h" > #include "main/mtypes.h" > +#include "main/imports.h" > #include "main/errors.h" > #include "main/shaderapi.h" > #include "main/uniforms.h" > @@ -83,33 +84,18 @@ st_nir_fixup_varying_slots(struct st_context *st, struct > exec_list *var_list) > static void > st_nir_assign_vs_in_locations(struct gl_program *prog, nir_shader *nir) > { > - unsigned attr, num_inputs = 0; > - unsigned input_to_index[VERT_ATTRIB_MAX] = {0}; > - > - /* TODO de-duplicate w/ similar code in st_translate_vertex_program()? */ > - for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) { > - if ((prog->info.inputs_read & BITFIELD64_BIT(attr)) != 0) { > - input_to_index[attr] = num_inputs; > - num_inputs++; > - if ((prog->DualSlotInputs & BITFIELD64_BIT(attr)) != 0) { > - /* add placeholder for second part of a double attribute */ > - num_inputs++; > - } > - } else { > - input_to_index[attr] = ~0; > - } > - } > - > - /* bit of a hack, mirroring st_translate_vertex_program */ > - input_to_index[VERT_ATTRIB_EDGEFLAG] = num_inputs; > - > nir->num_inputs = 0; > nir_foreach_variable_safe(var, &nir->inputs) { > - attr = var->data.location; > - assert(attr < ARRAY_SIZE(input_to_index)); > - > - if (input_to_index[attr] != ~0u) { > - var->data.driver_location = input_to_index[attr]; > + /* NIR already assigns dual-slot inputs to two locations so all we have > + * to do is compact everything down. > + */ > + if (var->data.location == VERT_ATTRIB_EDGEFLAG) { > + /* bit of a hack, mirroring st_translate_vertex_program */ > + var->data.driver_location = > _mesa_bitcount_64(nir->info.inputs_read); > + } else if (nir->info.inputs_read & BITFIELD64_BIT(var->data.location)) > { > + var->data.driver_location = > + _mesa_bitcount_64(nir->info.inputs_read & > + BITFIELD64_MASK(var->data.location)); > nir->num_inputs++; > } else { > /* Move unused input variables to the globals list (with no > @@ -743,6 +729,15 @@ st_link_nir(struct gl_context *ctx, > > nir_shader_gather_info(nir, nir_shader_get_entrypoint(nir)); > shader->Program->info = nir->info; > + if (i == MESA_SHADER_VERTEX) { > + /* NIR expands dual-slot inputs out to two locations. We need to > + * compact things back down GL-style single-slot inputs to avoid > + * confusing the state tracker. > + */ > + shader->Program->info.inputs_read = > + nir_get_single_slot_attribs_mask(nir->info.inputs_read, > + > shader->Program->DualSlotInputs); > + } > > if (prev != -1) { > struct gl_program *prev_shader = > -- > 2.17.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev