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

2016-03-15 Thread Samuel Pitoiset 
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

2016-03-16 Thread 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 
---
  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

2016-03-19 Thread 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 
---
  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

2016-03-19 Thread Pierre Moreau 
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