Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders

[Iago Toral]

On Wed, 2015-10-14 at 16:18 +0300, Francisco Jerez wrote:
> "Lofstedt, Marta"  writes:
> 
> > I have found a couple of more places in linker.cpp where we loop up to 
> > MESA_SHADER_FRAGMENT.
> > Should these now also be up to MESA_SHADER_COMPUTE instead?
> >
> Some might be oversights like this, but I guess in some cases a loop up
> to MESA_SHADER_FRAGMENT might be the right thing to do when doing stuff
> like linking varyings that really only applies to the graphics pipeline
> and not to compute shaders.

Right, I imagine that if any of these should also include the CS stage
we'll probably know via broken CS tests.

Iago

> > /Marta
> >
> >> -Original Message-
> >> From: Marta Lofstedt [mailto:marta.lofst...@linux.intel.com]
> >> Sent: Wednesday, October 14, 2015 2:56 PM
> >> To: mesa-dev@lists.freedesktop.org
> >> Cc: Lofstedt, Marta; Marta Lofstedt
> >> Subject: [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute
> >> shaders
> >> 
> >> From: Marta Lofstedt 
> >> 
> >> The split of Uniform blocks and shader storage block only loops up to
> >> MESA_SHADER_FRAGMENT and igonres compute shaders.
> >> This cause segfault when running the OpenGL ES 3.1 CTS tests with
> >> GL_ARB_compute_shader enabled.
> >> 
> >> Signed-off-by: Marta Lofstedt 
> >> ---
> >>  src/glsl/linker.cpp | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 
> >> c61c76e..5b5d6e6
> >> 100644
> >> --- a/src/glsl/linker.cpp
> >> +++ b/src/glsl/linker.cpp
> >> @@ -4392,7 +4392,7 @@ link_shaders(struct gl_context *ctx, struct
> >> gl_shader_program *prog)
> >>  * for gl_shader_program and gl_shader, so that drivers that need 
> >> separate
> >>  * index spaces for each set can have that.
> >>  */
> >> -   for (unsigned i = MESA_SHADER_VERTEX; i <=
> >> MESA_SHADER_FRAGMENT; i++) {
> >> +   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_COMPUTE;
> >> i++)
> >
> > Remove new line.
> >
> >> + {
> >>if (prog->_LinkedShaders[i] != NULL) {
> >>   gl_shader *sh = prog->_LinkedShaders[i];
> >>   split_ubos_and_ssbos(sh,
> >> --
> >> 2.1.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders

[Lofstedt, Marta]

> -Original Message-
> From: Francisco Jerez [mailto:curroje...@riseup.net]
> Sent: Wednesday, October 14, 2015 3:18 PM
> To: Lofstedt, Marta; Marta Lofstedt; mesa-dev@lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO
> also for compute shaders
> 
> "Lofstedt, Marta"  writes:
> 
> > I have found a couple of more places in linker.cpp where we loop up to
> MESA_SHADER_FRAGMENT.
> > Should these now also be up to MESA_SHADER_COMPUTE instead?
> >
> Some might be oversights like this, but I guess in some cases a loop up to
> MESA_SHADER_FRAGMENT might be the right thing to do when doing stuff
> like linking varyings that really only applies to the graphics pipeline and 
> not to
> compute shaders.
> 
Thanks Curro,

I will leave the potential other cases for now and send up a V2 with 
MESA_SHADER_STAGES instead.
This is blocking GLES 3.1 testing at the moment.

/Marta

> 
> > /Marta
> >
> >> -Original Message-
> >> From: Marta Lofstedt [mailto:marta.lofst...@linux.intel.com]
> >> Sent: Wednesday, October 14, 2015 2:56 PM
> >> To: mesa-dev@lists.freedesktop.org
> >> Cc: Lofstedt, Marta; Marta Lofstedt
> >> Subject: [PATCH] glsl: Enable split of lower UBOs and SSBO also for
> >> compute shaders
> >>
> >> From: Marta Lofstedt 
> >>
> >> The split of Uniform blocks and shader storage block only loops up to
> >> MESA_SHADER_FRAGMENT and igonres compute shaders.
> >> This cause segfault when running the OpenGL ES 3.1 CTS tests with
> >> GL_ARB_compute_shader enabled.
> >>
> >> Signed-off-by: Marta Lofstedt 
> >> ---
> >>  src/glsl/linker.cpp | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index
> >> c61c76e..5b5d6e6
> >> 100644
> >> --- a/src/glsl/linker.cpp
> >> +++ b/src/glsl/linker.cpp
> >> @@ -4392,7 +4392,7 @@ link_shaders(struct gl_context *ctx, struct
> >> gl_shader_program *prog)
> >>  * for gl_shader_program and gl_shader, so that drivers that need
> separate
> >>  * index spaces for each set can have that.
> >>  */
> >> -   for (unsigned i = MESA_SHADER_VERTEX; i <=
> >> MESA_SHADER_FRAGMENT; i++) {
> >> +   for (unsigned i = MESA_SHADER_VERTEX; i <=
> MESA_SHADER_COMPUTE;
> >> i++)
> >
> > Remove new line.
> >
> >> + {
> >>if (prog->_LinkedShaders[i] != NULL) {
> >>   gl_shader *sh = prog->_LinkedShaders[i];
> >>   split_ubos_and_ssbos(sh,
> >> --
> >> 2.1.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders

[Marta Lofstedt]

From: Marta Lofstedt 

The split of Uniform blocks and shader storage block only loops
up to MESA_SHADER_FRAGMENT and igonres compute shaders.
This cause segfault when running the OpenGL ES 3.1 CTS tests
with GL_ARB_compute_shader enabled.

Signed-off-by: Marta Lofstedt 
---
 src/glsl/linker.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index c61c76e..5b5d6e6 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -4392,7 +4392,7 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
 * for gl_shader_program and gl_shader, so that drivers that need separate
 * index spaces for each set can have that.
 */
-   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_FRAGMENT; i++) {
+   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_COMPUTE; i++) {
   if (prog->_LinkedShaders[i] != NULL) {
  gl_shader *sh = prog->_LinkedShaders[i];
  split_ubos_and_ssbos(sh,
-- 
2.1.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders

[Francisco Jerez]

"Lofstedt, Marta"  writes:

> I have found a couple of more places in linker.cpp where we loop up to 
> MESA_SHADER_FRAGMENT.
> Should these now also be up to MESA_SHADER_COMPUTE instead?
>
Some might be oversights like this, but I guess in some cases a loop up
to MESA_SHADER_FRAGMENT might be the right thing to do when doing stuff
like linking varyings that really only applies to the graphics pipeline
and not to compute shaders.

> /Marta
>
>> -Original Message-
>> From: Marta Lofstedt [mailto:marta.lofst...@linux.intel.com]
>> Sent: Wednesday, October 14, 2015 2:56 PM
>> To: mesa-dev@lists.freedesktop.org
>> Cc: Lofstedt, Marta; Marta Lofstedt
>> Subject: [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute
>> shaders
>> 
>> From: Marta Lofstedt 
>> 
>> The split of Uniform blocks and shader storage block only loops up to
>> MESA_SHADER_FRAGMENT and igonres compute shaders.
>> This cause segfault when running the OpenGL ES 3.1 CTS tests with
>> GL_ARB_compute_shader enabled.
>> 
>> Signed-off-by: Marta Lofstedt 
>> ---
>>  src/glsl/linker.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index c61c76e..5b5d6e6
>> 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -4392,7 +4392,7 @@ link_shaders(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>>  * for gl_shader_program and gl_shader, so that drivers that need 
>> separate
>>  * index spaces for each set can have that.
>>  */
>> -   for (unsigned i = MESA_SHADER_VERTEX; i <=
>> MESA_SHADER_FRAGMENT; i++) {
>> +   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_COMPUTE;
>> i++)
>
> Remove new line.
>
>> + {
>>if (prog->_LinkedShaders[i] != NULL) {
>>   gl_shader *sh = prog->_LinkedShaders[i];
>>   split_ubos_and_ssbos(sh,
>> --
>> 2.1.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders

[Francisco Jerez]

Hi Marta,

Marta Lofstedt  writes:

> From: Marta Lofstedt 
>
> The split of Uniform blocks and shader storage block only loops
> up to MESA_SHADER_FRAGMENT and igonres compute shaders.
> This cause segfault when running the OpenGL ES 3.1 CTS tests
> with GL_ARB_compute_shader enabled.
>
> Signed-off-by: Marta Lofstedt 
> ---
>  src/glsl/linker.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index c61c76e..5b5d6e6 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -4392,7 +4392,7 @@ link_shaders(struct gl_context *ctx, struct 
> gl_shader_program *prog)
>  * for gl_shader_program and gl_shader, so that drivers that need separate
>  * index spaces for each set can have that.
>  */
> -   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_FRAGMENT; i++) {
> +   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_COMPUTE; i++) {

Any reason not to use MESA_SHADER_STAGES as upper bound here so this
doesn't break again if new shader stages are added?

>if (prog->_LinkedShaders[i] != NULL) {
>   gl_shader *sh = prog->_LinkedShaders[i];
>   split_ubos_and_ssbos(sh,
> -- 
> 2.1.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders

[Lofstedt, Marta]

I have found a couple of more places in linker.cpp where we loop up to 
MESA_SHADER_FRAGMENT.
Should these now also be up to MESA_SHADER_COMPUTE instead?

/Marta

> -Original Message-
> From: Marta Lofstedt [mailto:marta.lofst...@linux.intel.com]
> Sent: Wednesday, October 14, 2015 2:56 PM
> To: mesa-dev@lists.freedesktop.org
> Cc: Lofstedt, Marta; Marta Lofstedt
> Subject: [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute
> shaders
> 
> From: Marta Lofstedt 
> 
> The split of Uniform blocks and shader storage block only loops up to
> MESA_SHADER_FRAGMENT and igonres compute shaders.
> This cause segfault when running the OpenGL ES 3.1 CTS tests with
> GL_ARB_compute_shader enabled.
> 
> Signed-off-by: Marta Lofstedt 
> ---
>  src/glsl/linker.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index c61c76e..5b5d6e6
> 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -4392,7 +4392,7 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
>  * for gl_shader_program and gl_shader, so that drivers that need separate
>  * index spaces for each set can have that.
>  */
> -   for (unsigned i = MESA_SHADER_VERTEX; i <=
> MESA_SHADER_FRAGMENT; i++) {
> +   for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_COMPUTE;
> i++)

Remove new line.

> + {
>if (prog->_LinkedShaders[i] != NULL) {
>   gl_shader *sh = prog->_LinkedShaders[i];
>   split_ubos_and_ssbos(sh,
> --
> 2.1.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev