[Mesa-dev] [PATCH] nvc0/ir: be careful about propagating very large offsets into const load

[Ilia Mirkin]

On Mon, Jan 11, 2016 at 4:46 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Indirect constbuf indexing works by using very large offsets. However if
> an indirect constbuf index load is const-propagated, it becomes a very
> large const offset. Take that into account when legalizing the SSA by
> moving the high parts of that offset into the file index. Also disallow
> very large (or small) indices on most other instructions.
>
> This fixes regressions in ubo_array_indexing/*-two-arrays piglit tests.
>
> Fixes: abd326e81b (nv50/ir: propagate indirect loads into instructions)
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp |  6 ++++++
>  src/gallium/drivers/nouveau/codegen/nv50_ir_target.h          |  2 +-
>  src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp   | 10 ++++++++++
>  src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.h     |  2 ++
>  4 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> index 64edffa..3806213 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -540,6 +540,12 @@ NVC0LegalizePostRA::visit(BasicBlock *bb)
>           // It seems like barriers are never required for tessellation since
>           // the warp size is 32, and there are always at most 32 tcs threads.
>           bb->remove(i);
> +      } else
> +      if (i->op == OP_LOAD && i->subOp == NV50_IR_SUBOP_LDC_IS) {
> +         int offset = i->src(0).get()->reg.data.offset;
> +         if (abs(offset) > 0x10000)
> +            i->src(0).get()->reg.fileIndex += offset >> 16;
> +         i->src(0).get()->reg.data.offset = (int)(short)offset;
>        } else {
>           // TODO: Move this to before register allocation for operations that
>           // need the $c register !
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h
> index 673f881..e6e1912 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h
> @@ -192,7 +192,7 @@ public:
>     virtual bool insnCanLoad(const Instruction *insn, int s,
>                              const Instruction *ld) const = 0;
>     virtual bool insnCanLoadOffset(const Instruction *insn, int s,
> -                                  int offset) const { return true; }
> +                                  int offset) const = 0;
>     virtual bool isOpSupported(operation, DataType) const = 0;
>     virtual bool isAccessSupported(DataFile, DataType) const = 0;
>     virtual bool isModSupported(const Instruction *,
> 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 014c652..e40547c 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
> @@ -384,6 +384,16 @@ TargetNVC0::insnCanLoad(const Instruction *i, int s,
>  }
>
>  bool
> +TargetNVC0::insnCanLoadOffset(const Instruction *insn, int s, int offset) const
> +{
> +   const ValueRef& ref = insn->src(s);
> +   if (ref.getFile() == FILE_MEMORY_CONST &&
> +       (insn->op != OP_LOAD || insn->subOp != NV50_IR_SUBOP_LDC_IS))
> +      return offset > -0x7fff && offset < 0x8000;

errrr... that should be

offset >= -0x8000 && offset < 0x8000.

> +   return true;
> +}
> +
> +bool
>  TargetNVC0::isAccessSupported(DataFile file, DataType ty) const
>  {
>     if (ty == TYPE_NONE)
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.h
> index 3c5c748..7d11cd9 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.h
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.h
> @@ -48,6 +48,8 @@ public:
>
>     virtual bool insnCanLoad(const Instruction *insn, int s,
>                              const Instruction *ld) const;
> +   virtual bool insnCanLoadOffset(const Instruction *insn, int s,
> +                                  int offset) const;
>     virtual bool isOpSupported(operation, DataType) const;
>     virtual bool isAccessSupported(DataFile, DataType) const;
>     virtual bool isModSupported(const Instruction *, int s, Modifier) const;
> --
> 2.4.10
>