[Mesa-dev] [PATCH 2/3] radeonsi/nir: Set vs_inputs_dual_locations and let NIR do the remap
Timothy Arceri
tarceri at itsqueeze.com
Thu Sep 6 03:51:16 UTC 2018
On 06/09/18 13:16, Jason Ekstrand wrote:
> On September 5, 2018 19:46:06 Timothy Arceri <tarceri at itsqueeze.com> wrote:
>
>> On 01/09/18 13:11, Jason Ekstrand 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));
>>
>> I'm probably missing something obvious but wont this always set
>> driver_location to 1?
>
> That's BITSET_MASK, not BITSET_BIT
Right thanks.
The series seems fine to me. I couldn't test on radeonsi as piglit is
causing issues with my setup currently but if anything is broken we
can fix it later. For the series:
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
>
>>
>>> 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 =
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list