[Mesa-dev] [PATCH 1/2] nvir/gm107: iterate over all defs in SchedDataCalculatorGM107::findFirstUse

[Samuel Pitoiset]

On 12/05/2017 12:01 PM, Karol Herbst wrote:
> In the sched data calculator we have to track first use of defs by iterating
> over all defs of an instruction, not just the first one.
> 
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>   .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 26 ++++++++++++----------
>   1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> index af7e2d4fd8..db1585818c 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> @@ -3974,18 +3974,20 @@ SchedDataCalculatorGM107::findFirstUse(const Instruction *bari) const
>   
>         for (int s = 0; insn->srcExists(s); ++s) {
>            const Value *src = insn->src(s).rep();
> -         if (bari->def(0).getFile() == FILE_GPR) {
> -            if (insn->src(s).getFile() != FILE_GPR ||
> -                src->reg.data.id + src->reg.size / 4 - 1 < minGPR ||
> -                src->reg.data.id > maxGPR)
> -               continue;
> -            return insn;
> -         } else
> -         if (bari->def(0).getFile() == FILE_PREDICATE) {
> -            if (insn->src(s).getFile() != FILE_PREDICATE ||
> -                src->reg.data.id != minGPR)
> -               continue;
> -            return insn;
> +         for (int d = 0; bari->defExists(d); ++d) {
> +            if (bari->def(d).getFile() == FILE_GPR) {
> +               if (insn->src(s).getFile() != FILE_GPR ||
> +                   src->reg.data.id + src->reg.size / 4 - 1 < minGPR ||
> +                   src->reg.data.id > maxGPR)
> +                  continue;
> +               return insn;
> +            } else
> +            if (bari->def(d).getFile() == FILE_PREDICATE) {
> +               if (insn->src(s).getFile() != FILE_PREDICATE ||
> +                   src->reg.data.id != minGPR)

This looks wrong to me, you are comparing minGPR of def(0) but you if 
have more than one defs... (same for the FILE_GPR case).

Would be definitely better to move the computation of minGPR/maxGPR in 
the defs loop.

> +                  continue;
> +               return insn;
> +            }
>            }
>         }
>      }
>