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

[Samuel Pitoiset]

On 03/16/2016 10:38 AM, Hans de Goede wrote:
> 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.

Yes currently it is unused, but maybe this will be useful for images.

>
>
>>         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 ?

No, because auxCBSlot is 0 for compute on Kepler.

>
> Regards,
>
> Hans