[Mesa-dev] [PATCH] anv/pipeline: Only consider double elements which actually exist

[Lionel Landwerlin]

On 05/09/2018 11:24, Lionel Landwerlin wrote:
> On 05/09/2018 00:04, Jason Ekstrand wrote:
>> The brw_vs_prog_data::double_inputs_read field comes directly from
>> shader_info::double_inputs which may contain inputs which are not
>> actually read.  Instead of using it directly, AND it with inputs_read
>> which is only things which are read.  Otherwise, we may end up
>> subtracting too many elements when computing elem_count.
>>
>> Cc: mesa-stable at lists.freedesktop.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103241
>
>
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>


Actually, I would like to take that back for a minute.

As far as I can tell, the content of inputs_read & double_inputs_read 
are copied directly from what gets computed in NIR.
For VS stage, both variable seems to be set a set in lock step in 
src/compiler/nir/nir_gather_info.c
So I would expect the following assertion to be true : 
(vs_prog_data->double_inputs_read & vs_prog_data->inputs_read) == 
vs_prog_data->double_inputs_read

Isn't the problem here that inputs_read get compacted in 
src/compiler/nir/nir_linking_helpers.c and double_inputs_read isn't 
updated accordingly.
If that's case, this change might not actually fix the bug.

-
Lionel


>
>
>> ---
>>   src/intel/vulkan/genX_pipeline.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/intel/vulkan/genX_pipeline.c 
>> b/src/intel/vulkan/genX_pipeline.c
>> index b531205508c..297e9455b51 100644
>> --- a/src/intel/vulkan/genX_pipeline.c
>> +++ b/src/intel/vulkan/genX_pipeline.c
>> @@ -91,7 +91,8 @@ emit_vertex_input(struct anv_pipeline *pipeline,
>>        /* Pull inputs_read out of the VS prog data */
>>      const uint64_t inputs_read = vs_prog_data->inputs_read;
>> -   const uint64_t double_inputs_read = 
>> vs_prog_data->double_inputs_read;
>> +   const uint64_t double_inputs_read =
>> +      vs_prog_data->double_inputs_read & inputs_read;
>>      assert((inputs_read & ((1 << VERT_ATTRIB_GENERIC0) - 1)) == 0);
>>      const uint32_t elements = inputs_read >> VERT_ATTRIB_GENERIC0;
>>      const uint32_t elements_double = double_inputs_read >> 
>> VERT_ATTRIB_GENERIC0;
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev