[Mesa-dev] [PATCH 2/5] nv50/ir: make use of auxCBSlot instead of magic numbers
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 --- 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); 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; case SV_SAMPLE_INDEX: // TODO: Properly pass source as an address in the PIX address space -- 2.7.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] nv50/ir: make use of auxCBSlot instead of magic numbers
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 --- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] nv50/ir: make use of auxCBSlot instead of magic numbers
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 --- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] nv50/ir: make use of auxCBSlot instead of magic numbers
Hello, On 10:38 AM - Mar 16 2016, 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 > >--- > > 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. On Fermi+, the global memory is united, but it is not the case on Tesla. So the slot remains needed for Tesla. (Tesla will keep hunting you! :-p) Global and const memory are not completely the same, so I'm somewhat reluctant to reuse the same variable slot for both of them. But, I'm just giving my 2cents here, so feel free to keep as is. Regardless of your pick, this is Acked-by: Pierre Moreau > > > >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 ? Why is it changing functionality? This code is only run for GK104+, in which case `prog->driver->io.auxCBSlot == 0`, cf patch 1. Regards, Pierre > > Regards, > > Hans > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev