On Sat, Aug 13, 2016 at 1:24 PM, karol herbst <karolher...@gmail.com> wrote:
> 2016-08-13 18:17 GMT+02:00 Ilia Mirkin <imir...@alum.mit.edu>:
>> On Sat, Aug 13, 2016 at 6:02 AM, Karol Herbst <karolher...@gmail.com> wrote:
>>> no changes without a dual_issue pass
>>>
>>> changes with for ./GpuTest /test=pixmark_piano /benchmark /no_scorebox
>>> /msaa=0
>>> /benchmark_duration_ms=60000 /width=1024 /height=640:
>>>
>>> inst_executed: 1.03G
>>> inst_issued1: 538M -> 535M
>>> inst_issued2: 251M -> 254M
>>>
>>> score: 1038 -> 1052
>>>
>>> Signed-off-by: Karol Herbst <karolher...@gmail.com>
>>> ---
>>> src/gallium/drivers/nouveau/codegen/nv50_ir.cpp | 11
>>> +++++++++--
>>> src/gallium/drivers/nouveau/codegen/nv50_ir.h | 1 +
>>> src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp | 4 ++++
>>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
>>> index 179ad0b..7a90cb7 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
>>> @@ -893,12 +893,19 @@ insnCheckCommutationDefDef(const Instruction *a,
>>> const Instruction *b)
>>> bool
>>> Instruction::isCommutationLegal(const Instruction *i) const
>>> {
>>> - bool ret = insnCheckCommutationDefDef(this, i);
>>> - ret = ret && insnCheckCommutationDefSrc(this, i);
>>> + bool ret = !i->dependsOn(this);
>>> ret = ret && insnCheckCommutationDefSrc(i, this);
>>
>> This condition is covered by your new dependsOn() call. Did you mean
>> dependsOn to have something different? I haven't really thought
>> carefully about this yet.
>
> dependsOn should just check whether the second instruction depend on
> the first one, where isCommutationLegal also checks if you could swap
> both.
>
> mad $r1 $r2 $r3 $r4
> add $r4 $r2 $r2
>
> =>
>
> add.isCommutationLegal(mad) : false
> add.dependsOn(mad) : false
>
> those instructions can be dual issued, but the isCommutationLegal
> method was too strict about the check.
>
> I hope this is enough to clear it up.
I think you missed my point.
dependsOn == !(DefDef(this, i) && DefSrc(i, this))
But then you check DefSrc(i, this) again in isCommutationLegal, so it becomes
!dependsOn && DefSrc(i, this), i.e.
DefDef(this, i) && DefSrc(i, this) && DefSrc(i, this)
Either you meant a different condition, or there's redundant code.
>
>>
>>> return ret;
>>> }
>>>
>>> +bool
>>> +Instruction::dependsOn(const Instruction *i) const
>>> +{
>>> + bool ret = insnCheckCommutationDefDef(this, i);
>>> + ret = ret && insnCheckCommutationDefSrc(i, this);
>>> + return !ret;
>>> +}
>>> +
>>> TexInstruction::TexInstruction(Function *fn, operation op)
>>> : Instruction(fn, op, TYPE_F32)
>>> {
>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>>> index 6d2ee8b..d81fca9 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
>>> @@ -831,6 +831,7 @@ public:
>>> bool isDead() const;
>>> bool isNop() const;
>>> bool isCommutationLegal(const Instruction *) const; // must be adjacent
>>> !
>>> + bool dependsOn(const Instruction *) const; // weaker form of
>>> isCommutationLegal
>>> bool isActionEqual(const Instruction *) const;
>>> bool isResultEqual(const Instruction *) const;
>>>
>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
>>> index faf2121..8ce8c19 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
>>> @@ -620,6 +620,10 @@ bool TargetNVC0::canDualIssue(const Instruction *a,
>>> const Instruction *b) const
>>> // not if the 2nd instruction isn't necessarily executed
>>> if (clA == OPCLASS_TEXTURE || clA == OPCLASS_FLOW)
>>> return false;
>>> +
>>> + if (b->dependsOn(a))
>>> + return false;
>>> +
>>> // anything with MOV
>>> if (a->op == OP_MOV || b->op == OP_MOV)
>>> return true;
>>> --
>>> 2.9.2
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
