Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
On Thu, May 26, 2016 at 7:09 PM, Ian Romanick wrote: > On 05/26/2016 03:39 PM, Ilia Mirkin wrote: >> On Thu, May 26, 2016 at 6:26 PM, Ian Romanick wrote: >>> On 05/26/2016 03:03 PM, Ilia Mirkin wrote: This will cause st/mesa to break, no? Right now validate_io iterates over the shader ir, which st/mesa frees after linking. >>> >>> Only as much as it is already broken. :) Any desktop OpenGL application >>> using GLSL ES shaders would already have that problem. I suspect there >>> are more of those than there are applications using debug contexts. I >> >> Doesn't glretrace create a debug context? Maybe I'm just imagining >> things. One would have to do SSO in ES to observe the breaks right >> now, which I think is pretty rare. > > I don't know much of anything about glretrace. It would make sense for > it to at least have an option to create a debug context. > >>> believe this should all be cleared up after patch 6 in this series which >>> deletes the old validate_io function. >> >> Yeah... I'm just hoping this can get ordered after that patch. Or at >> least be in the same series. > > I put the original version of this patch at this point in the series so > that I wouldn't have to put ES checking in the next patch. In fact, > that was the patch's raison d'ĂȘtre. I plan to land all of these > together, and I don't think any of them will get cherry-picked to > earlier releases. I can rearrange them if you /really/ want me to. :) As long as we don't end up with a situation where st/mesa-based drivers can't use glretrace, I'm happy. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
On Thu, 2016-05-26 at 14:59 -0700, Ian Romanick wrote: > From: Ian Romanick > > Signed-off-by: Ian Romanick > Suggested-by: Timothy Arceri > Cc: Timothy Arceri Looks good to me. Reviewed-by: Timothy Arceri ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
On 05/26/2016 03:39 PM, Ilia Mirkin wrote: > On Thu, May 26, 2016 at 6:26 PM, Ian Romanick wrote: >> On 05/26/2016 03:03 PM, Ilia Mirkin wrote: >>> This will cause st/mesa to break, no? Right now validate_io iterates >>> over the shader ir, which st/mesa frees after linking. >> >> Only as much as it is already broken. :) Any desktop OpenGL application >> using GLSL ES shaders would already have that problem. I suspect there >> are more of those than there are applications using debug contexts. I > > Doesn't glretrace create a debug context? Maybe I'm just imagining > things. One would have to do SSO in ES to observe the breaks right > now, which I think is pretty rare. I don't know much of anything about glretrace. It would make sense for it to at least have an option to create a debug context. >> believe this should all be cleared up after patch 6 in this series which >> deletes the old validate_io function. > > Yeah... I'm just hoping this can get ordered after that patch. Or at > least be in the same series. I put the original version of this patch at this point in the series so that I wouldn't have to put ES checking in the next patch. In fact, that was the patch's raison d'ĂȘtre. I plan to land all of these together, and I don't think any of them will get cherry-picked to earlier releases. I can rearrange them if you /really/ want me to. :) > -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
On Thu, May 26, 2016 at 6:26 PM, Ian Romanick wrote: > On 05/26/2016 03:03 PM, Ilia Mirkin wrote: >> This will cause st/mesa to break, no? Right now validate_io iterates >> over the shader ir, which st/mesa frees after linking. > > Only as much as it is already broken. :) Any desktop OpenGL application > using GLSL ES shaders would already have that problem. I suspect there > are more of those than there are applications using debug contexts. I Doesn't glretrace create a debug context? Maybe I'm just imagining things. One would have to do SSO in ES to observe the breaks right now, which I think is pretty rare. > believe this should all be cleared up after patch 6 in this series which > deletes the old validate_io function. Yeah... I'm just hoping this can get ordered after that patch. Or at least be in the same series. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
On 05/26/2016 03:03 PM, Ilia Mirkin wrote: > This will cause st/mesa to break, no? Right now validate_io iterates > over the shader ir, which st/mesa frees after linking. Only as much as it is already broken. :) Any desktop OpenGL application using GLSL ES shaders would already have that problem. I suspect there are more of those than there are applications using debug contexts. I believe this should all be cleared up after patch 6 in this series which deletes the old validate_io function. > -ilia > > On Thu, May 26, 2016 at 5:59 PM, Ian Romanick wrote: >> From: Ian Romanick >> >> Signed-off-by: Ian Romanick >> Suggested-by: Timothy Arceri >> Cc: Timothy Arceri >> --- >> src/mesa/main/pipelineobj.c| 18 -- >> src/mesa/main/shader_query.cpp | 12 >> 2 files changed, 20 insertions(+), 10 deletions(-) >> >> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c >> index b150198..5a46cfe 100644 >> --- a/src/mesa/main/pipelineobj.c >> +++ b/src/mesa/main/pipelineobj.c >> @@ -906,7 +906,8 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, >> /* Validate inputs against outputs, this cannot be done during linking >> * since programs have been linked separately from each other. >> * >> -* From OpenGL 4.5 Core spec: >> +* Section 11.1.3.11 (Validation) of the OpenGL 4.5 Core Profile spec >> says: >> +* >> * "Separable program objects may have validation failures that >> cannot be >> * detected without the complete program pipeline. Mismatched >> interfaces, >> * improper usage of program objects together, and the same >> @@ -914,8 +915,21 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, >> * program objects." >> * >> * OpenGL ES 3.1 specification has the same text. >> +* >> +* Section 11.1.3.11 (Validation) of the OpenGL ES spec also says: >> +* >> +*An INVALID_OPERATION error is generated by any command that >> transfers >> +*vertices to the GL or launches compute work if the current set of >> +*active program objects cannot be executed, for reasons including: >> +* >> +** The current program pipeline object contains a shader interface >> +* that doesn't have an exact match (see section 7.4.1) >> +* >> +* Based on this, only perform the most-strict checking on ES or when the >> +* application has created a debug context. >> */ >> - if (!_mesa_validate_pipeline_io(pipe)) >> + if ((_mesa_is_gles(ctx) || (ctx->Const.ContextFlags & >> GL_CONTEXT_FLAG_DEBUG_BIT)) && >> + !_mesa_validate_pipeline_io(pipe)) >>return GL_FALSE; >> >> pipe->Validated = GL_TRUE; >> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp >> index 9e18a1c..a37c179 100644 >> --- a/src/mesa/main/shader_query.cpp >> +++ b/src/mesa/main/shader_query.cpp >> @@ -1372,7 +1372,7 @@ _mesa_get_program_resourceiv(struct gl_shader_program >> *shProg, >> >> static bool >> validate_io(const struct gl_shader *producer, >> -const struct gl_shader *consumer, bool isES) >> +const struct gl_shader *consumer) >> { >> assert(producer && consumer); >> unsigned inputs = 0, outputs = 0; >> @@ -1416,10 +1416,6 @@ validate_io(const struct gl_shader *producer, >> * packing makes this challenging. >> */ >> >> - /* Currently no matching done for desktop. */ >> - if (!isES) >> - return true; >> - >> /* For each output in a, find input in b and do any required checks. */ >> foreach_in_list(ir_instruction, out, producer->ir) { >>ir_variable *out_var = out->as_variable(); >> @@ -1502,9 +1498,9 @@ _mesa_validate_pipeline_io(struct gl_pipeline_object >> *pipeline) >> break; >> >> if (!validate_io(shProg[prev]->_LinkedShaders[prev], >> - shProg[idx]->_LinkedShaders[idx], >> - shProg[prev]->IsES || shProg[idx]->IsES)) >> -return false; >> + shProg[idx]->_LinkedShaders[idx])) >> + return false; >> + >> prev = idx; >>} >> } >> -- >> 2.5.5 >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
This will cause st/mesa to break, no? Right now validate_io iterates over the shader ir, which st/mesa frees after linking. -ilia On Thu, May 26, 2016 at 5:59 PM, Ian Romanick wrote: > From: Ian Romanick > > Signed-off-by: Ian Romanick > Suggested-by: Timothy Arceri > Cc: Timothy Arceri > --- > src/mesa/main/pipelineobj.c| 18 -- > src/mesa/main/shader_query.cpp | 12 > 2 files changed, 20 insertions(+), 10 deletions(-) > > diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c > index b150198..5a46cfe 100644 > --- a/src/mesa/main/pipelineobj.c > +++ b/src/mesa/main/pipelineobj.c > @@ -906,7 +906,8 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, > /* Validate inputs against outputs, this cannot be done during linking > * since programs have been linked separately from each other. > * > -* From OpenGL 4.5 Core spec: > +* Section 11.1.3.11 (Validation) of the OpenGL 4.5 Core Profile spec > says: > +* > * "Separable program objects may have validation failures that > cannot be > * detected without the complete program pipeline. Mismatched > interfaces, > * improper usage of program objects together, and the same > @@ -914,8 +915,21 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, > * program objects." > * > * OpenGL ES 3.1 specification has the same text. > +* > +* Section 11.1.3.11 (Validation) of the OpenGL ES spec also says: > +* > +*An INVALID_OPERATION error is generated by any command that > transfers > +*vertices to the GL or launches compute work if the current set of > +*active program objects cannot be executed, for reasons including: > +* > +** The current program pipeline object contains a shader interface > +* that doesn't have an exact match (see section 7.4.1) > +* > +* Based on this, only perform the most-strict checking on ES or when the > +* application has created a debug context. > */ > - if (!_mesa_validate_pipeline_io(pipe)) > + if ((_mesa_is_gles(ctx) || (ctx->Const.ContextFlags & > GL_CONTEXT_FLAG_DEBUG_BIT)) && > + !_mesa_validate_pipeline_io(pipe)) >return GL_FALSE; > > pipe->Validated = GL_TRUE; > diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp > index 9e18a1c..a37c179 100644 > --- a/src/mesa/main/shader_query.cpp > +++ b/src/mesa/main/shader_query.cpp > @@ -1372,7 +1372,7 @@ _mesa_get_program_resourceiv(struct gl_shader_program > *shProg, > > static bool > validate_io(const struct gl_shader *producer, > -const struct gl_shader *consumer, bool isES) > +const struct gl_shader *consumer) > { > assert(producer && consumer); > unsigned inputs = 0, outputs = 0; > @@ -1416,10 +1416,6 @@ validate_io(const struct gl_shader *producer, > * packing makes this challenging. > */ > > - /* Currently no matching done for desktop. */ > - if (!isES) > - return true; > - > /* For each output in a, find input in b and do any required checks. */ > foreach_in_list(ir_instruction, out, producer->ir) { >ir_variable *out_var = out->as_variable(); > @@ -1502,9 +1498,9 @@ _mesa_validate_pipeline_io(struct gl_pipeline_object > *pipeline) > break; > > if (!validate_io(shProg[prev]->_LinkedShaders[prev], > - shProg[idx]->_LinkedShaders[idx], > - shProg[prev]->IsES || shProg[idx]->IsES)) > -return false; > + shProg[idx]->_LinkedShaders[idx])) > + return false; > + > prev = idx; >} > } > -- > 2.5.5 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context
From: Ian Romanick Signed-off-by: Ian Romanick Suggested-by: Timothy Arceri Cc: Timothy Arceri --- src/mesa/main/pipelineobj.c| 18 -- src/mesa/main/shader_query.cpp | 12 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c index b150198..5a46cfe 100644 --- a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c @@ -906,7 +906,8 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, /* Validate inputs against outputs, this cannot be done during linking * since programs have been linked separately from each other. * -* From OpenGL 4.5 Core spec: +* Section 11.1.3.11 (Validation) of the OpenGL 4.5 Core Profile spec says: +* * "Separable program objects may have validation failures that cannot be * detected without the complete program pipeline. Mismatched interfaces, * improper usage of program objects together, and the same @@ -914,8 +915,21 @@ _mesa_validate_program_pipeline(struct gl_context* ctx, * program objects." * * OpenGL ES 3.1 specification has the same text. +* +* Section 11.1.3.11 (Validation) of the OpenGL ES spec also says: +* +*An INVALID_OPERATION error is generated by any command that transfers +*vertices to the GL or launches compute work if the current set of +*active program objects cannot be executed, for reasons including: +* +** The current program pipeline object contains a shader interface +* that doesn't have an exact match (see section 7.4.1) +* +* Based on this, only perform the most-strict checking on ES or when the +* application has created a debug context. */ - if (!_mesa_validate_pipeline_io(pipe)) + if ((_mesa_is_gles(ctx) || (ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) && + !_mesa_validate_pipeline_io(pipe)) return GL_FALSE; pipe->Validated = GL_TRUE; diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index 9e18a1c..a37c179 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -1372,7 +1372,7 @@ _mesa_get_program_resourceiv(struct gl_shader_program *shProg, static bool validate_io(const struct gl_shader *producer, -const struct gl_shader *consumer, bool isES) +const struct gl_shader *consumer) { assert(producer && consumer); unsigned inputs = 0, outputs = 0; @@ -1416,10 +1416,6 @@ validate_io(const struct gl_shader *producer, * packing makes this challenging. */ - /* Currently no matching done for desktop. */ - if (!isES) - return true; - /* For each output in a, find input in b and do any required checks. */ foreach_in_list(ir_instruction, out, producer->ir) { ir_variable *out_var = out->as_variable(); @@ -1502,9 +1498,9 @@ _mesa_validate_pipeline_io(struct gl_pipeline_object *pipeline) break; if (!validate_io(shProg[prev]->_LinkedShaders[prev], - shProg[idx]->_LinkedShaders[idx], - shProg[prev]->IsES || shProg[idx]->IsES)) -return false; + shProg[idx]->_LinkedShaders[idx])) + return false; + prev = idx; } } -- 2.5.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev