[Mesa-dev] [PATCH 3/6] nv50/ir: replace post_ra_dead by Instruction::isDead

[Karol Herbst]

2016-10-08 18:39 GMT+02:00 Samuel Pitoiset <samuel.pitoiset at gmail.com>:
>
>
> On 10/08/2016 05:43 PM, Karol Herbst wrote:
>>
>> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
>> ---
>>  src/gallium/drivers/nouveau/codegen/nv50_ir.h        |  2 +-
>>  .../drivers/nouveau/codegen/nv50_ir_peephole.cpp     | 20
>> +++++++-------------
>>  2 files changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> index bedbdcc..9d1a72f 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>> @@ -829,7 +829,7 @@ public:
>>     }
>>
>>     inline bool isPseudo() const { return op < OP_MOV; }
>> -   bool isDead() const;
>> +   bool isDead(bool postRA = false) const;
>>     bool isNop() const;
>>     bool isCommutationLegal(const Instruction *) const; // must be
>> adjacent !
>>     bool isActionEqual(const Instruction *) const;
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> index 780820f..3fcadd9 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> @@ -61,7 +61,7 @@ Instruction::isNop() const
>>     return false;
>>  }
>>
>> -bool Instruction::isDead() const
>> +bool Instruction::isDead(bool postRA) const
>>  {
>
>
> You might want to do :
>
> if (postRA)
>   return post_ra_dead(this);
>
> which doesn't require any changes above.

post_ra_dead is basically broken, we were just lucky it worked out so far.

>
>>     if (op == OP_STORE ||
>>         op == OP_EXPORT ||
>> @@ -70,8 +70,11 @@ bool Instruction::isDead() const
>>         op == OP_WRSV)
>>        return false;
>>
>> +   if (postRA && op == OP_MOV && subOp == NV50_IR_SUBOP_MOV_FINAL)
>> +      return false;
>
>
> Why do you need that specific case?

those mov defs have a refcount of 0, so they would get eliminated
otherwise, but shouldn't.

>
>> +
>>     for (int d = 0; defExists(d); ++d)
>> -      if (getDef(d)->refCount() || getDef(d)->reg.data.id >= 0)
>> +      if (getDef(d)->refCount() || (!postRA && getDef(d)->reg.data.id >=
>> 0))
>>           return false;
>>
>>     if (terminator || asFlow())
>> @@ -2959,15 +2962,6 @@ private:
>>     virtual bool visit(BasicBlock *);
>>  };
>>
>> -static bool
>> -post_ra_dead(Instruction *i)
>> -{
>> -   for (int d = 0; i->defExists(d); ++d)
>> -      if (i->getDef(d)->refCount())
>> -         return false;
>> -   return true;
>> -}
>> -
>>  bool
>>  NV50PostRaConstantFolding::visit(BasicBlock *bb)
>>  {
>> @@ -3014,13 +3008,13 @@ NV50PostRaConstantFolding::visit(BasicBlock *bb)
>>              /* There's no post-RA dead code elimination, so do it here
>>               * XXX: if we add more code-removing post-RA passes, we might
>>               *      want to create a post-RA dead-code elim pass */
>> -            if (post_ra_dead(vtmp->getInsn())) {
>> +            if (vtmp->getInsn()->isDead(true)) {
>>                 Value *src = vtmp->getInsn()->getSrc(0);
>>                 // Careful -- splits will have already been removed from
>> the
>>                 // functions. Don't double-delete.
>>                 if (vtmp->getInsn()->bb)
>>                    delete_Instruction(prog, vtmp->getInsn());
>> -               if (src->getInsn() && post_ra_dead(src->getInsn()))
>> +               if (src->getInsn() && src->getInsn()->isDead(true))
>>                    delete_Instruction(prog, src->getInsn());
>>              }
>>
>>
>