Re: [Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context

[Ilia Mirkin]

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

[Timothy Arceri]

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

[Ian Romanick]

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

[Ilia Mirkin]

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

[Ian Romanick]

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

[Ilia Mirkin]

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

[Ian Romanick]

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