[Mesa-dev] [PATCH 05/15] i965/fs: Don't dead code eliminate CMP(N).

[Matt Turner]

On Mon, Oct 28, 2013 at 11:08 PM, Eric Anholt <eric at anholt.net> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> Since compare instructions write the flag register, they should not be
>> considered dead even if their destination is never read. Instead of
>> removing them if found to be dead, set their destination to null to free
>> a register.
>>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 7e7ced0..b985251 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -1903,10 +1903,12 @@ fs_visitor::dead_code_eliminate()
>>
>>           if (dead) {
>>              /* Don't dead code eliminate instructions that write to the
>> -             * accumulator as a side-effect. Instead just set the destination
>> -             * to the null register to free it.
>> +             * accumulator or flag register as a side effect. Instead just set
>> +             * the destination to the null register to free it.
>>               */
>>              switch (inst->opcode) {
>> +            case BRW_OPCODE_CMP:
>> +            case BRW_OPCODE_CMPN:
>>              case BRW_OPCODE_ADDC:
>>              case BRW_OPCODE_SUBB:
>>              case BRW_OPCODE_MACH:
>
> I don't think this one is needed in the series any more after the rework
> to not generate new CMP.whatever.f0 to a non-null destination in CSE,
> but I think it should still land anyway.  I'd prefer the test for
> "writes to the flag register" to be inst->writes_flag() rather than just
> a couple of opcodes that always write the flag, though.

That's a good point and a good idea. I'll drop it from this series and
send it using inst->writes_flag() separately.