Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-19 Thread Hans de Goede

Hi,

On 14-03-16 21:50, Samuel Pitoiset wrote:




Btw, do you need someone with commit access to push your previous
series (the tgsi thing)? I can do this for you.


Thanks for the offer. IIRC Ilia wanted some minor fixes there, so I'll do
a v2 tomorrow. Talking about commit rights, I guess it would be
convenient for all if I would get commit rights myself? I promise I won't
push anythings without acks.


Yes sure, I trust you, no worries. :-)



I already have a freedesktop.org account, my username is jwrdegoede.


Please open a ticket on bugs.freedesktop to ask for commit rights.


Done:

https://bugs.freedesktop.org/show_bug.cgi?id=94594

Can you or Ilia please ack this ?

Thanks,

Hans
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-19 Thread Samuel Pitoiset



On 03/17/2016 05:07 PM, Hans de Goede wrote:

Hi,

On 14-03-16 21:50, Samuel Pitoiset wrote:




Btw, do you need someone with commit access to push your previous
series (the tgsi thing)? I can do this for you.


Thanks for the offer. IIRC Ilia wanted some minor fixes there, so
I'll do
a v2 tomorrow. Talking about commit rights, I guess it would be
convenient for all if I would get commit rights myself? I promise I
won't
push anythings without acks.


Yes sure, I trust you, no worries. :-)



I already have a freedesktop.org account, my username is jwrdegoede.


Please open a ticket on bugs.freedesktop to ask for commit rights.


Done:

https://bugs.freedesktop.org/show_bug.cgi?id=94594

Can you or Ilia please ack this ?



Done.


Thanks,

Hans

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-14 Thread Samuel Pitoiset



On 03/14/2016 08:50 PM, Hans de Goede wrote:

Hi,

On 14-03-16 16:41, Samuel Pitoiset wrote:



On 03/14/2016 04:28 PM, Hans de Goede wrote:

Hi,

On 14-03-16 16:05, Ilia Mirkin wrote:

There's a less hacky and more hacky way forward. The more hacky
solution is
to set file index to -1 or something and then not do the lowering when
you
see that.

The less hacky solution is the one you proposed as #1 - introduce a new
file for "buffer" memory and lower it to the global file by adding a
base
offset.

Right now the meaning of global is overloaded - before lowering it
implicitly includes the buffer vase address, and after lowering, it
explicitly includes it. Splitting it out I to another file type seems
like
the cleaner way forward, not sure what issue you were seeing with that
approach.


Ok.


I agree with you guys, the solution #1 is fine by me.

Btw, do you need someone with commit access to push your previous
series (the tgsi thing)? I can do this for you.


Thanks for the offer. IIRC Ilia wanted some minor fixes there, so I'll do
a v2 tomorrow. Talking about commit rights, I guess it would be
convenient for all if I would get commit rights myself? I promise I won't
push anythings without acks.


Yes sure, I trust you, no worries. :-)



I already have a freedesktop.org account, my username is jwrdegoede.


Please open a ticket on bugs.freedesktop to ask for commit rights.



Regards,

Hans









 > (I didn't understand your argument about potential future

issues.)


There was not much to understand, it is just something I worried about,
but was not sure if there actually was something to worry about :)

If you feel that solution #1 (which was also my first hunch) is
the right one then I will go and implement that.


What I really don't want is to somehow differentiate glsl-sourced
and opencl-sourced compute programs in the backend.


Ok, understood.

Regards,

Hans



On Mar 14, 2016 6:22 AM, "Hans de Goede"  wrote:


This little "hack" fixes the use of OpenCL global memory buffers with
nouveau, but clearly the #if 0 is not a solution as it breaks buffers
with GLSL.

The reason I'm posting this as an RFC patch is to discuss how to solve
this properly, 2 solutions come to mind:

1) Use separate nv50_ir::FILE_MEMORY_xxx values for buffers versus
TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL, looking at
translateFile()
we currently have:

case TGSI_FILE_BUFFER:  return
nv50_ir::FILE_MEMORY_GLOBAL;
case TGSI_FILE_MEMORY:  return
nv50_ir::FILE_MEMORY_GLOBAL;

So doing a
s/nv50_ir::FILE_MEMORY_GLOBAL/nv50_ir::FILE_MEMORY_BUFFER/
everywhere and then adding a new FILE_MEMORY_GLOBAL seems like an
obvious fix.

But I'm afraid that we will have similar issues with OpenCL using
flat addresses where as GLSL will have some implied base-address /
offset in other places too, which brings me to solution 2:

2) Add a flag to Program to indicate that it is an OpenCL compute
kernel;
or possible use a different Program::TYPE_* for OpenCL ?

I've a feeling that this is what we want since the addressing
models
are just different and we likely will need to implement different
behavior
in various places based on this.

This will also allow us to use INPUT and CONST in tgsi code build
from
OpenCL programs and use that flag to do the right thing, rather
then
introducing new MEMORY[x], INPUT resp. MEMORY[x], CONST
declarations
for this.

I'm esp. worried that once GLSL gets global support it will want
different behavior for TGSI_FILE_MEMORY with
TGSI_MEMORY_TYPE_GLOBAL
then OpenCL, just like things are now with buffers, rendering
solution
1. a non solution

So I'm seeking input on how to move forward with this ...  ?

Signed-off-by: Hans de Goede 
---
  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 4

  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 ++
  2 files changed, 6 insertions(+)

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 de0c72b..15012ac 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -1525,6 +1525,10 @@ Converter::makeSym(uint tgsiFile, int fileIdx,
int
idx, int c, uint32_t address)

 if (tgsiFile == TGSI_FILE_MEMORY) {
switch (code->memoryFiles[fileIdx].mem_type) {
+  case TGSI_MEMORY_TYPE_GLOBAL:
+ /* No-op this is the default for TGSI_FILE_MEMORY */
+ sym->setFile(FILE_MEMORY_GLOBAL);
+ break;
case TGSI_MEMORY_TYPE_SHARED:
   sym->setFile(FILE_MEMORY_SHARED);
   break;
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 6cb4dd4..bcc96de 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/

Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-14 Thread Hans de Goede

Hi,

On 14-03-16 16:41, Samuel Pitoiset wrote:



On 03/14/2016 04:28 PM, Hans de Goede wrote:

Hi,

On 14-03-16 16:05, Ilia Mirkin wrote:

There's a less hacky and more hacky way forward. The more hacky
solution is
to set file index to -1 or something and then not do the lowering when
you
see that.

The less hacky solution is the one you proposed as #1 - introduce a new
file for "buffer" memory and lower it to the global file by adding a base
offset.

Right now the meaning of global is overloaded - before lowering it
implicitly includes the buffer vase address, and after lowering, it
explicitly includes it. Splitting it out I to another file type seems
like
the cleaner way forward, not sure what issue you were seeing with that
approach.


Ok.


I agree with you guys, the solution #1 is fine by me.

Btw, do you need someone with commit access to push your previous series (the 
tgsi thing)? I can do this for you.


Thanks for the offer. IIRC Ilia wanted some minor fixes there, so I'll do
a v2 tomorrow. Talking about commit rights, I guess it would be
convenient for all if I would get commit rights myself? I promise I won't
push anythings without acks.

I already have a freedesktop.org account, my username is jwrdegoede.

Regards,

Hans









 > (I didn't understand your argument about potential future

issues.)


There was not much to understand, it is just something I worried about,
but was not sure if there actually was something to worry about :)

If you feel that solution #1 (which was also my first hunch) is
the right one then I will go and implement that.


What I really don't want is to somehow differentiate glsl-sourced
and opencl-sourced compute programs in the backend.


Ok, understood.

Regards,

Hans



On Mar 14, 2016 6:22 AM, "Hans de Goede"  wrote:


This little "hack" fixes the use of OpenCL global memory buffers with
nouveau, but clearly the #if 0 is not a solution as it breaks buffers
with GLSL.

The reason I'm posting this as an RFC patch is to discuss how to solve
this properly, 2 solutions come to mind:

1) Use separate nv50_ir::FILE_MEMORY_xxx values for buffers versus
TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL, looking at
translateFile()
we currently have:

case TGSI_FILE_BUFFER:  return nv50_ir::FILE_MEMORY_GLOBAL;
case TGSI_FILE_MEMORY:  return nv50_ir::FILE_MEMORY_GLOBAL;

So doing a
s/nv50_ir::FILE_MEMORY_GLOBAL/nv50_ir::FILE_MEMORY_BUFFER/
everywhere and then adding a new FILE_MEMORY_GLOBAL seems like an
obvious fix.

But I'm afraid that we will have similar issues with OpenCL using
flat addresses where as GLSL will have some implied base-address /
offset in other places too, which brings me to solution 2:

2) Add a flag to Program to indicate that it is an OpenCL compute
kernel;
or possible use a different Program::TYPE_* for OpenCL ?

I've a feeling that this is what we want since the addressing models
are just different and we likely will need to implement different
behavior
in various places based on this.

This will also allow us to use INPUT and CONST in tgsi code build
from
OpenCL programs and use that flag to do the right thing, rather then
introducing new MEMORY[x], INPUT resp. MEMORY[x], CONST declarations
for this.

I'm esp. worried that once GLSL gets global support it will want
different behavior for TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL
then OpenCL, just like things are now with buffers, rendering
solution
1. a non solution

So I'm seeking input on how to move forward with this ...  ?

Signed-off-by: Hans de Goede 
---
  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 4 
  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 ++
  2 files changed, 6 insertions(+)

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 de0c72b..15012ac 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -1525,6 +1525,10 @@ Converter::makeSym(uint tgsiFile, int fileIdx,
int
idx, int c, uint32_t address)

 if (tgsiFile == TGSI_FILE_MEMORY) {
switch (code->memoryFiles[fileIdx].mem_type) {
+  case TGSI_MEMORY_TYPE_GLOBAL:
+ /* No-op this is the default for TGSI_FILE_MEMORY */
+ sym->setFile(FILE_MEMORY_GLOBAL);
+ break;
case TGSI_MEMORY_TYPE_SHARED:
   sym->setFile(FILE_MEMORY_SHARED);
   break;
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 6cb4dd4..bcc96de 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -2106,6 +2106,7 @@ NVC0LoweringPass::visit(Instruction *i)
} else if (i->src(0).getFile() == FILE_SHADER_OUT

Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-14 Thread Samuel Pitoiset



On 03/14/2016 04:28 PM, Hans de Goede wrote:

Hi,

On 14-03-16 16:05, Ilia Mirkin wrote:

There's a less hacky and more hacky way forward. The more hacky
solution is
to set file index to -1 or something and then not do the lowering when
you
see that.

The less hacky solution is the one you proposed as #1 - introduce a new
file for "buffer" memory and lower it to the global file by adding a base
offset.

Right now the meaning of global is overloaded - before lowering it
implicitly includes the buffer vase address, and after lowering, it
explicitly includes it. Splitting it out I to another file type seems
like
the cleaner way forward, not sure what issue you were seeing with that
approach.


Ok.


I agree with you guys, the solution #1 is fine by me.

Btw, do you need someone with commit access to push your previous series 
(the tgsi thing)? I can do this for you.




 > (I didn't understand your argument about potential future

issues.)


There was not much to understand, it is just something I worried about,
but was not sure if there actually was something to worry about :)

If you feel that solution #1 (which was also my first hunch) is
the right one then I will go and implement that.


What I really don't want is to somehow differentiate glsl-sourced
and opencl-sourced compute programs in the backend.


Ok, understood.

Regards,

Hans



On Mar 14, 2016 6:22 AM, "Hans de Goede"  wrote:


This little "hack" fixes the use of OpenCL global memory buffers with
nouveau, but clearly the #if 0 is not a solution as it breaks buffers
with GLSL.

The reason I'm posting this as an RFC patch is to discuss how to solve
this properly, 2 solutions come to mind:

1) Use separate nv50_ir::FILE_MEMORY_xxx values for buffers versus
TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL, looking at
translateFile()
we currently have:

case TGSI_FILE_BUFFER:  return nv50_ir::FILE_MEMORY_GLOBAL;
case TGSI_FILE_MEMORY:  return nv50_ir::FILE_MEMORY_GLOBAL;

So doing a
s/nv50_ir::FILE_MEMORY_GLOBAL/nv50_ir::FILE_MEMORY_BUFFER/
everywhere and then adding a new FILE_MEMORY_GLOBAL seems like an
obvious fix.

But I'm afraid that we will have similar issues with OpenCL using
flat addresses where as GLSL will have some implied base-address /
offset in other places too, which brings me to solution 2:

2) Add a flag to Program to indicate that it is an OpenCL compute
kernel;
or possible use a different Program::TYPE_* for OpenCL ?

I've a feeling that this is what we want since the addressing models
are just different and we likely will need to implement different
behavior
in various places based on this.

This will also allow us to use INPUT and CONST in tgsi code build
from
OpenCL programs and use that flag to do the right thing, rather then
introducing new MEMORY[x], INPUT resp. MEMORY[x], CONST declarations
for this.

I'm esp. worried that once GLSL gets global support it will want
different behavior for TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL
then OpenCL, just like things are now with buffers, rendering
solution
1. a non solution

So I'm seeking input on how to move forward with this ...  ?

Signed-off-by: Hans de Goede 
---
  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 4 
  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 ++
  2 files changed, 6 insertions(+)

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 de0c72b..15012ac 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -1525,6 +1525,10 @@ Converter::makeSym(uint tgsiFile, int fileIdx,
int
idx, int c, uint32_t address)

 if (tgsiFile == TGSI_FILE_MEMORY) {
switch (code->memoryFiles[fileIdx].mem_type) {
+  case TGSI_MEMORY_TYPE_GLOBAL:
+ /* No-op this is the default for TGSI_FILE_MEMORY */
+ sym->setFile(FILE_MEMORY_GLOBAL);
+ break;
case TGSI_MEMORY_TYPE_SHARED:
   sym->setFile(FILE_MEMORY_SHARED);
   break;
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 6cb4dd4..bcc96de 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -2106,6 +2106,7 @@ NVC0LoweringPass::visit(Instruction *i)
} else if (i->src(0).getFile() == FILE_SHADER_OUTPUT) {
   assert(prog->getType() ==
Program::TYPE_TESSELLATION_CONTROL);
   i->op = OP_VFETCH;
+#if 0
} else if (i->src(0).getFile() == FILE_MEMORY_GLOBAL) {
   Value *ind = i->getIndirect(0, 1);
   Value *ptr = loadResInfo64(ind, i->getSrc(0)->reg.fileIndex *
16);
@@ -2126,6 +2127,7 @@ NVC0LoweringPass::visit(Instruction *i)
   if 

Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-14 Thread Hans de Goede

Hi,

On 14-03-16 16:05, Ilia Mirkin wrote:

There's a less hacky and more hacky way forward. The more hacky solution is
to set file index to -1 or something and then not do the lowering when you
see that.

The less hacky solution is the one you proposed as #1 - introduce a new
file for "buffer" memory and lower it to the global file by adding a base
offset.

Right now the meaning of global is overloaded - before lowering it
implicitly includes the buffer vase address, and after lowering, it
explicitly includes it. Splitting it out I to another file type seems like
the cleaner way forward, not sure what issue you were seeing with that
approach.


Ok.

> (I didn't understand your argument about potential future

issues.)


There was not much to understand, it is just something I worried about,
but was not sure if there actually was something to worry about :)

If you feel that solution #1 (which was also my first hunch) is
the right one then I will go and implement that.


What I really don't want is to somehow differentiate glsl-sourced
and opencl-sourced compute programs in the backend.


Ok, understood.

Regards,

Hans



On Mar 14, 2016 6:22 AM, "Hans de Goede"  wrote:


This little "hack" fixes the use of OpenCL global memory buffers with
nouveau, but clearly the #if 0 is not a solution as it breaks buffers
with GLSL.

The reason I'm posting this as an RFC patch is to discuss how to solve
this properly, 2 solutions come to mind:

1) Use separate nv50_ir::FILE_MEMORY_xxx values for buffers versus
TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL, looking at
translateFile()
we currently have:

case TGSI_FILE_BUFFER:  return nv50_ir::FILE_MEMORY_GLOBAL;
case TGSI_FILE_MEMORY:  return nv50_ir::FILE_MEMORY_GLOBAL;

So doing a s/nv50_ir::FILE_MEMORY_GLOBAL/nv50_ir::FILE_MEMORY_BUFFER/
everywhere and then adding a new FILE_MEMORY_GLOBAL seems like an
obvious fix.

But I'm afraid that we will have similar issues with OpenCL using
flat addresses where as GLSL will have some implied base-address /
offset in other places too, which brings me to solution 2:

2) Add a flag to Program to indicate that it is an OpenCL compute kernel;
or possible use a different Program::TYPE_* for OpenCL ?

I've a feeling that this is what we want since the addressing models
are just different and we likely will need to implement different
behavior
in various places based on this.

This will also allow us to use INPUT and CONST in tgsi code build from
OpenCL programs and use that flag to do the right thing, rather then
introducing new MEMORY[x], INPUT resp. MEMORY[x], CONST declarations
for this.

I'm esp. worried that once GLSL gets global support it will want
different behavior for TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL
then OpenCL, just like things are now with buffers, rendering solution
1. a non solution

So I'm seeking input on how to move forward with this ...  ?

Signed-off-by: Hans de Goede 
---
  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 4 
  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 ++
  2 files changed, 6 insertions(+)

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 de0c72b..15012ac 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -1525,6 +1525,10 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
idx, int c, uint32_t address)

 if (tgsiFile == TGSI_FILE_MEMORY) {
switch (code->memoryFiles[fileIdx].mem_type) {
+  case TGSI_MEMORY_TYPE_GLOBAL:
+ /* No-op this is the default for TGSI_FILE_MEMORY */
+ sym->setFile(FILE_MEMORY_GLOBAL);
+ break;
case TGSI_MEMORY_TYPE_SHARED:
   sym->setFile(FILE_MEMORY_SHARED);
   break;
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 6cb4dd4..bcc96de 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -2106,6 +2106,7 @@ NVC0LoweringPass::visit(Instruction *i)
} else if (i->src(0).getFile() == FILE_SHADER_OUTPUT) {
   assert(prog->getType() == Program::TYPE_TESSELLATION_CONTROL);
   i->op = OP_VFETCH;
+#if 0
} else if (i->src(0).getFile() == FILE_MEMORY_GLOBAL) {
   Value *ind = i->getIndirect(0, 1);
   Value *ptr = loadResInfo64(ind, i->getSrc(0)->reg.fileIndex *
16);
@@ -2126,6 +2127,7 @@ NVC0LoweringPass::visit(Instruction *i)
   if (i->defExists(0)) {
  bld.mkMov(i->getDef(0), bld.mkImm(0));
   }
+#endif
}
break;
 case OP_ATOM:
--
2.7.2





___
Nouveau mailing list
Nou

Re: [Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-14 Thread Ilia Mirkin
There's a less hacky and more hacky way forward. The more hacky solution is
to set file index to -1 or something and then not do the lowering when you
see that.

The less hacky solution is the one you proposed as #1 - introduce a new
file for "buffer" memory and lower it to the global file by adding a base
offset.

Right now the meaning of global is overloaded - before lowering it
implicitly includes the buffer vase address, and after lowering, it
explicitly includes it. Splitting it out I to another file type seems like
the cleaner way forward, not sure what issue you were seeing with that
approach. (I didn't understand your argument about potential future
issues.) What I really don't want is to somehow differentiate glsl-sourced
and opencl-sourced compute programs in the backend.
On Mar 14, 2016 6:22 AM, "Hans de Goede"  wrote:

> This little "hack" fixes the use of OpenCL global memory buffers with
> nouveau, but clearly the #if 0 is not a solution as it breaks buffers
> with GLSL.
>
> The reason I'm posting this as an RFC patch is to discuss how to solve
> this properly, 2 solutions come to mind:
>
> 1) Use separate nv50_ir::FILE_MEMORY_xxx values for buffers versus
>TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL, looking at
> translateFile()
>we currently have:
>
>case TGSI_FILE_BUFFER:  return nv50_ir::FILE_MEMORY_GLOBAL;
>case TGSI_FILE_MEMORY:  return nv50_ir::FILE_MEMORY_GLOBAL;
>
>So doing a s/nv50_ir::FILE_MEMORY_GLOBAL/nv50_ir::FILE_MEMORY_BUFFER/
>everywhere and then adding a new FILE_MEMORY_GLOBAL seems like an
>obvious fix.
>
>But I'm afraid that we will have similar issues with OpenCL using
>flat addresses where as GLSL will have some implied base-address /
>offset in other places too, which brings me to solution 2:
>
> 2) Add a flag to Program to indicate that it is an OpenCL compute kernel;
>or possible use a different Program::TYPE_* for OpenCL ?
>
>I've a feeling that this is what we want since the addressing models
>are just different and we likely will need to implement different
> behavior
>in various places based on this.
>
>This will also allow us to use INPUT and CONST in tgsi code build from
>OpenCL programs and use that flag to do the right thing, rather then
>introducing new MEMORY[x], INPUT resp. MEMORY[x], CONST declarations
>for this.
>
>I'm esp. worried that once GLSL gets global support it will want
>different behavior for TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL
>then OpenCL, just like things are now with buffers, rendering solution
>1. a non solution
>
> So I'm seeking input on how to move forward with this ...  ?
>
> Signed-off-by: Hans de Goede 
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 4 
>  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 ++
>  2 files changed, 6 insertions(+)
>
> 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 de0c72b..15012ac 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -1525,6 +1525,10 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
> idx, int c, uint32_t address)
>
> if (tgsiFile == TGSI_FILE_MEMORY) {
>switch (code->memoryFiles[fileIdx].mem_type) {
> +  case TGSI_MEMORY_TYPE_GLOBAL:
> + /* No-op this is the default for TGSI_FILE_MEMORY */
> + sym->setFile(FILE_MEMORY_GLOBAL);
> + break;
>case TGSI_MEMORY_TYPE_SHARED:
>   sym->setFile(FILE_MEMORY_SHARED);
>   break;
> 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 6cb4dd4..bcc96de 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -2106,6 +2106,7 @@ NVC0LoweringPass::visit(Instruction *i)
>} else if (i->src(0).getFile() == FILE_SHADER_OUTPUT) {
>   assert(prog->getType() == Program::TYPE_TESSELLATION_CONTROL);
>   i->op = OP_VFETCH;
> +#if 0
>} else if (i->src(0).getFile() == FILE_MEMORY_GLOBAL) {
>   Value *ind = i->getIndirect(0, 1);
>   Value *ptr = loadResInfo64(ind, i->getSrc(0)->reg.fileIndex *
> 16);
> @@ -2126,6 +2127,7 @@ NVC0LoweringPass::visit(Instruction *i)
>   if (i->defExists(0)) {
>  bld.mkMov(i->getDef(0), bld.mkImm(0));
>   }
> +#endif
>}
>break;
> case OP_ATOM:
> --
> 2.7.2
>
>
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [RFC mesa] nouveau: Add support for OpenCL global memory buffers

2016-03-14 Thread Hans de Goede
This little "hack" fixes the use of OpenCL global memory buffers with
nouveau, but clearly the #if 0 is not a solution as it breaks buffers
with GLSL.

The reason I'm posting this as an RFC patch is to discuss how to solve
this properly, 2 solutions come to mind:

1) Use separate nv50_ir::FILE_MEMORY_xxx values for buffers versus
   TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL, looking at translateFile()
   we currently have:

   case TGSI_FILE_BUFFER:  return nv50_ir::FILE_MEMORY_GLOBAL;
   case TGSI_FILE_MEMORY:  return nv50_ir::FILE_MEMORY_GLOBAL;

   So doing a s/nv50_ir::FILE_MEMORY_GLOBAL/nv50_ir::FILE_MEMORY_BUFFER/
   everywhere and then adding a new FILE_MEMORY_GLOBAL seems like an
   obvious fix.

   But I'm afraid that we will have similar issues with OpenCL using
   flat addresses where as GLSL will have some implied base-address /
   offset in other places too, which brings me to solution 2:

2) Add a flag to Program to indicate that it is an OpenCL compute kernel;
   or possible use a different Program::TYPE_* for OpenCL ?

   I've a feeling that this is what we want since the addressing models
   are just different and we likely will need to implement different behavior
   in various places based on this.

   This will also allow us to use INPUT and CONST in tgsi code build from
   OpenCL programs and use that flag to do the right thing, rather then
   introducing new MEMORY[x], INPUT resp. MEMORY[x], CONST declarations
   for this.

   I'm esp. worried that once GLSL gets global support it will want
   different behavior for TGSI_FILE_MEMORY with TGSI_MEMORY_TYPE_GLOBAL
   then OpenCL, just like things are now with buffers, rendering solution
   1. a non solution

So I'm seeking input on how to move forward with this ...  ?

Signed-off-by: Hans de Goede 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 4 
 src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 ++
 2 files changed, 6 insertions(+)

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 de0c72b..15012ac 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -1525,6 +1525,10 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, 
int c, uint32_t address)
 
if (tgsiFile == TGSI_FILE_MEMORY) {
   switch (code->memoryFiles[fileIdx].mem_type) {
+  case TGSI_MEMORY_TYPE_GLOBAL:
+ /* No-op this is the default for TGSI_FILE_MEMORY */
+ sym->setFile(FILE_MEMORY_GLOBAL);
+ break;
   case TGSI_MEMORY_TYPE_SHARED:
  sym->setFile(FILE_MEMORY_SHARED);
  break;
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 6cb4dd4..bcc96de 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -2106,6 +2106,7 @@ NVC0LoweringPass::visit(Instruction *i)
   } else if (i->src(0).getFile() == FILE_SHADER_OUTPUT) {
  assert(prog->getType() == Program::TYPE_TESSELLATION_CONTROL);
  i->op = OP_VFETCH;
+#if 0
   } else if (i->src(0).getFile() == FILE_MEMORY_GLOBAL) {
  Value *ind = i->getIndirect(0, 1);
  Value *ptr = loadResInfo64(ind, i->getSrc(0)->reg.fileIndex * 16);
@@ -2126,6 +2127,7 @@ NVC0LoweringPass::visit(Instruction *i)
  if (i->defExists(0)) {
 bld.mkMov(i->getDef(0), bld.mkImm(0));
  }
+#endif
   }
   break;
case OP_ATOM:
-- 
2.7.2

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau