Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders
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
> -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
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. > /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
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
Re: [Mesa-dev] [PATCH] glsl: Enable split of lower UBOs and SSBO also for compute shaders
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
[Mesa-dev] [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++) { 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