On 11/28/2016 09:12 PM, Timothy Arceri wrote:
> On Mon, 2016-11-28 at 18:59 -0800, Ian Romanick wrote:
>> On 11/20/2016 05:28 AM, Timothy Arceri wrote:
>>>
>>> Now that we have a linked_stages bitfield we can use this
>>> to check if the program is used at a later stage.
>>>
>>> This change is also required to be able to use gl_program
>>> rather than gl_shader_program in the CurrentProgram array.
>>> ---
>>> src/mesa/main/pipelineobj.c | 17 +++++++----------
>>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/mesa/main/pipelineobj.c
>>> b/src/mesa/main/pipelineobj.c
>>> index 35d416d..45131d2 100644
>>> --- a/src/mesa/main/pipelineobj.c
>>> +++ b/src/mesa/main/pipelineobj.c
>>> @@ -730,30 +730,27 @@ program_stages_all_active(struct
>>> gl_pipeline_object *pipe,
>>> static bool
>>> program_stages_interleaved_illegally(const struct
>>> gl_pipeline_object *pipe)
>>> {
>>> - struct gl_shader_program *prev = NULL;
>>> - unsigned i, j;
>>> + unsigned prev_linked_stages = 0;
>>>
>>> /* Look for programs bound to stages: A -> B -> A, with any
>>> intervening
>>> * sequence of unrelated programs or empty stages.
>>> */
>>> - for (i = 0; i < MESA_SHADER_STAGES; i++) {
>>> + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>>> struct gl_shader_program *cur = pipe->CurrentProgram[i];
>>>
>>> /* Empty stages anywhere in the pipe are OK */
>>> - if (!cur || cur == prev)
>>> + if (!cur || cur->data->linked_stages == prev_linked_stages)
>>
>> The previous 'cur == prev' check would prevent validation when, say,
>> a
>> program with a linked vertex shader and geometry shader were attached
>> to
>> a pipeline. Just looking at this line of code, it's not immediately
>> obvious why the new check is equivalent. I'm guessing that code
>> elsewhere generates an error if the application binds a programs with
>> the same set of stages to a pipeline
>>
>> glGenProgramPipelines(1, &pipe);
>> glGenPrograms(2, prog);
>>
>> // compile and link two programs that have a vertex & fragment
>> // shader.
>> // ...
>>
>> glBindProgramPipeline(pipe);
>> glUseProgramStages(pipe, GL_VERTEX_SHADER_BIT, prog[0]);
>> glUseProgramStages(pipe, GL_FRAGMENT_SHADER_BIT, prog[1]);
>>
>> In the existing code, this pipeline will get validated here (and
>> fail).
>> With the new code, it will not get validated here.
>
> Your guess was correct. The check above this
> one program_stages_all_active() ensures that all stages of a program
> are active so we will always fail before validating interleaved stages
> in the example you gave. Therefore cur->data->linked_stages ==
> prev_linked_stages can only ever mean that we are looking at the same
> program.
>
> I could add a comment if you think it would help?
Yeah. With that added, patches 1 and 2 are
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
>>>
>>> continue;
>>>
>>> - if (prev) {
>>> + if (prev_linked_stages) {
>>> /* We've seen an A -> B transition; look at the rest of
>>> the pipe
>>> * to see if we ever see A again.
>>> */
>>> - for (j = i + 1; j < MESA_SHADER_STAGES; j++) {
>>> - if (pipe->CurrentProgram[j] == prev)
>>> - return true;
>>> - }
>>> + if (prev_linked_stages >> (i + 1))
>>> + return true;
>>> }
>>>
>>> - prev = cur;
>>> + prev_linked_stages = cur->data->linked_stages;
>>> }
>>>
>>> return false;
>>>
>>
>> _______________________________________________
>> 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
