[Mesa-dev] [PATCH 7/9] i965/fs: Handle VS inputs in the NIR backend.

[Connor Abbott]

On Wed, Mar 11, 2015 at 6:28 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> I think I mentioned this at least once before, but I don't think that this
> is a good long-term solution.  The nir_lower_io function does two things
> primarily.  1) It assigns each variable a "device location" and 2) changes
> the variable loads to loads with an offset.  In the long-term, I think we
> want to split 1) out and let the backend control what "device location" gets
> assigned to each input/output.  Then, add a knob to 2) to select how we want
> things packed.  Doing this will let us avoid this whole re-mapping mess and,
> in the case of uniforms, better control things so that we can push some of
> them.  That said, this is *not* a NAK of the current patch.  If you want to
> hold off on that for a while, this looks ok.
> --Jason

Yes, some re-thinking of how inputs/outputs/uniforms are handled would
certainly be useful. For some history, when I wrote the old code (and
Jason's code is essentially doing the same thing but in a
different/better way) it was more of a "let's get this working for
now" sort of thing since I wasn't sure what a cleaner solution would
look like (I hadn't even written the backend code yet!). But I agree
that scalar VS support shouldn't have to block on that rewrite.

>
> On Mon, Mar 9, 2015 at 1:58 AM, Kenneth Graunke <kenneth at whitecape.org>
> wrote:
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 3baafc4..1734d03 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -199,11 +199,32 @@ fs_visitor::nir_setup_inputs(nir_shader *shader)
>>     struct hash_entry *entry;
>>     hash_table_foreach(shader->inputs, entry) {
>>        nir_variable *var = (nir_variable *) entry->data;
>> +      enum brw_reg_type type = brw_type_for_base_type(var->type);
>>        fs_reg input = offset(nir_inputs, var->data.driver_location);
>>
>>        fs_reg reg;
>>        switch (stage) {
>> -      case MESA_SHADER_VERTEX:
>> +      case MESA_SHADER_VERTEX: {
>> +         /* Our ATTR file is indexed by VERT_ATTRIB_*, which is the value
>> +          * stored in nir_variable::location.
>> +          *
>> +          * However, NIR's load_input intrinsics use a different index -
>> an
>> +          * offset into a single contiguous array containing all inputs.
>> +          * This index corresponds to the nir_variable::driver_location
>> field.
>> +          *
>> +          * So, we need to copy from fs_reg(ATTR, var->location) to
>> +          * offset(nir_inputs, var->data.driver_location).
>> +          */
>> +         unsigned components = var->type->without_array()->components();
>> +         unsigned array_length = var->type->is_array() ?
>> var->type->length : 1;
>> +         for (unsigned i = 0; i < array_length; i++) {
>> +            for (unsigned j = 0; j < components; j++) {
>> +               emit(MOV(retype(offset(input, components * i + j), type),
>> +                        offset(fs_reg(ATTR, var->data.location + i,
>> type), j)));
>> +            }
>> +         }
>> +         break;
>> +      }
>>        case MESA_SHADER_GEOMETRY:
>>        case MESA_SHADER_COMPUTE:
>>           unreachable("fs_visitor not used for these stages yet.");
>> --
>> 2.2.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>