[Mesa-dev] [PATCH 1/9] glsl: don't lower fragdata array if the output data types don't match

[Samuel Iglesias Gonsálvez]

On 05/05/15 00:08, Matt Turner wrote:
> On Tue, Feb 24, 2015 at 10:02 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>
>> Commit 7e414b58640aee6e243d337e72cea290c354f632 broke the gl_FragData array
>> into separate gl_FragData[i] variables, so drivers can eliminate useless
>> writes to gl_FragData improving their performance.
>>
>> The problem occurs when GLSL IR code is linked in the following case:
>>
>> * The FS output variable base data type does not match gl_FragData one (float
>>   vector)
>> * The FS output variable is replaced by gl_out_FragDataX because of commit
>>   7e414b58640aee6 with X from 0 to GL_MAX_DRAW_BUFFERS.
>>
>> Then the FS output variable base data type is lost in the resulting GLSL IR,
>> making that the driver does a wrong assignment to gl_out_FragData components
>> because of unmatching data types.
>>
>> This patch reverts the fragdata array lowering when the output var base data type
>> doesn't match gl_out_FragData, i.e., when output variable base data type is
>> not a float or a float vector.
>>
>> This patch fixes 250 dEQP tests (tested in an Intel Haswell machine)
>>
>> dEQP-GLES3.functional.fragment_out.random.* (22 failed tests)
>> dEQP-GLES3.functional.fragment_out.array.uint.* (120 failed tests)
>> dEQP-GLES3.functional.fragment_out.array.int.* (108 failed tests)
> 
> All of these pass for me on Haswell with today's master branch --
> finally a case of NIR's screwing up the types helping something :)
> 
> I've confirmed that this generates the same code with non-NIR and NIR
> after this patch.
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> 
>>
>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>> ---
>>  src/glsl/opt_dead_builtin_varyings.cpp | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/src/glsl/opt_dead_builtin_varyings.cpp b/src/glsl/opt_dead_builtin_varyings.cpp
>> index 50c8aa7..4919c8b 100644
>> --- a/src/glsl/opt_dead_builtin_varyings.cpp
>> +++ b/src/glsl/opt_dead_builtin_varyings.cpp
>> @@ -99,6 +99,16 @@ public:
>>           }
>>           else {
>>              this->fragdata_usage |= 1 << index->get_uint_component(0);
>> +            /* Don't lowered fragdata array if the output variable
> 
> typo: lower

OK, I am going to fix this typo and apply your R-b.

Thanks!

Sam