[Mesa-dev] [PATCH] i965/vec4: check opcode on vec4_instruction::reads_flag(channel)

[Alejandro Piñeiro]

On 23/10/15 17:46, Kenneth Graunke wrote:
> On Friday, October 23, 2015 04:04:35 PM Alejandro Piñeiro wrote:
>> Commit f17b78 added an alternative reads_flag(channel) that returned
>> if the instruction was reading a specific channel flag. By mistake it
>> only took into account the predicate, but when the opcode is
>> VS_OPCODE_UNPACK_FLAGS_SIMD4X2 there isn't any predicate, but the flag
>> are used.
>>
>> That mistake caused some regressions on old hw. More information on
>> this bug:
>> https://bugs.freedesktop.org/show_bug.cgi?id=92621
>> ---
>>
>> The bug mentions G965, ILK and G45, but I only have available ILK to
>> test, so it would be good if someone confirms that the regressions
>> get solved on that hw too. Having said so, it would be strange if
>> it is not the case.
>>
>> Additionally, the bug mentions 24 regressions. But it only lists 21.
>> And I only detected 17 regressions on ILK. So it would be good
>> to confirm how many regressions the series introduced, and if
>> this new commit solve all of them.
>>
>>  src/mesa/drivers/dri/i965/brw_ir_vec4.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> index 74e9733..cc4104c 100644
>> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> @@ -188,8 +188,8 @@ public:
>>  
>>     bool reads_flag(unsigned c)
>>     {
>> -      if (!reads_flag())
>> -         return false;
>> +      if (opcode == VS_OPCODE_UNPACK_FLAGS_SIMD4X2)
>> +         return true;
>>  
>>        switch (predicate) {
>>        case BRW_PREDICATE_NONE:
>>
> Thanks, Alejandro!
>
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks to you for the quick review!

I have just pushed the commit. But as I mentioned before, I only have an
ILK to test the fix. So although I assume that this will solve the
problem on G45 and G965, it is still pending to confirm it.

BR

-- 
Alejandro Piñeiro (apinheiro at igalia.com)