[Mesa-dev] [PATCH 2/5] nv50/ir: make use of auxCBSlot instead of magic numbers

[Hans de Goede]

Hi,

On 15-03-16 21:55, Samuel Pitoiset wrote:
> This avoids using magic numbers for the driver constbuf slot which
> is always 15 except for compute shaders on gk104+ where the slot 0
> is used.
>
> For gk104+, some special compute-related values like the thread
> index are uploaded to screen->parm which is currently bound on c0.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp     | 3 ++-
>   src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> index d284446..4bebfdc 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -2178,7 +2178,8 @@ Converter::getResourceBase(const int r)
>
>      switch (r) {
>      case TGSI_RESOURCE_GLOBAL:
> -      sym = new_Symbol(prog, nv50_ir::FILE_MEMORY_GLOBAL, 15);
> +      sym = new_Symbol(prog, nv50_ir::FILE_MEMORY_GLOBAL,
> +                       info->io.auxCBSlot);

Note this is dead code, see patch 6/6 of the patch-set I just send.

Also do we need to specify a slot here at all? The new code paths
to re-enable global mem with clover do not use this and work fine
in my testing on a gk107.


>         break;
>      case TGSI_RESOURCE_LOCAL:
>         assert(prog->getType() == Program::TYPE_COMPUTE);
> 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 d879339..e0af4c0 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -1698,7 +1698,8 @@ NVC0LoweringPass::handleRDSV(Instruction *i)
>         }
>         addr += prog->driver->prop.cp.gridInfoBase;
>         bld.mkLoad(TYPE_U32, i->getDef(0),
> -                 bld.mkSymbol(FILE_MEMORY_CONST, 0, TYPE_U32, addr), NULL);
> +                 bld.mkSymbol(FILE_MEMORY_CONST, prog->driver->io.auxCBSlot,
> +                              TYPE_U32, addr), NULL);
>         break;

You're changing functionality here not just replacing a magic number,
the commit msg does not reflect this. Maybe do this in a separate patch ?

Regards,

Hans