[Mesa-dev] [PATCH 1/2] nvc0/ir: avoid generating illegal instructions for compute constbuf loads

[Hans de Goede]

Hi,

On 26-05-16 04:44, Ilia Mirkin wrote:
> For user-supplied constbufs, fileIndex is 0. In that case, when we
> subtract 1, we'll end up loading from constbuf offset -16. This is
> illegal, and there are asserts to avoid it. Normally we'd just DCE it,
> but no point in generating the instructions if they're not going to be
> used.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 3 ++-
>  1 file changed, 2 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 869040c..da2fa4b 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -2180,11 +2180,11 @@ NVC0LoweringPass::handleLDST(Instruction *i)
>           // memory.
>           int8_t fileIndex = i->getSrc(0)->reg.fileIndex - 1;
>           Value *ind = i->getIndirect(0, 1);
> -         Value *ptr = loadUboInfo64(ind, fileIndex * 16);
>
>           // TODO: clamp the offset to the maximum number of const buf.
>           if (i->src(0).isIndirect(1)) {
>              Value *offset = bld.loadImm(NULL, i->getSrc(0)->reg.data.offset + typeSizeof(i->sType));
> +            Value *ptr = loadUboInfo64(ind, fileIndex * 16);
>              Value *length = loadUboLength32(ind, fileIndex * 16);
>              Value *pred = new_LValue(func, FILE_PREDICATE);
>              if (i->src(0).isIndirect(0)) {
> @@ -2200,6 +2200,7 @@ NVC0LoweringPass::handleLDST(Instruction *i)
>                 bld.mkMov(i->getDef(0), bld.mkImm(0));
>              }
>           } else if (fileIndex >= 0) {
> +            Value *ptr = loadUboInfo64(ind, fileIndex * 16);
>              if (i->src(0).isIndirect(0)) {
>                 bld.mkOp2(OP_ADD, TYPE_U64, ptr, ptr, i->getIndirect(0, 0));
>              }
>

This patch does not seem to actually change anything, you've just moved the exact
same declaration to 2 places ... ?

Regards,

Hans