[Mesa-dev] [PATCH 2/3] mesa: build up a mask of active shader stages in a pipeline

Tapani Pälli tapani.palli at intel.com
Mon Dec 7 23:39:12 PST 2015


On 12/08/2015 07:28 AM, Timothy Arceri wrote:
> On Tue, 2015-12-08 at 07:09 +0200, Tapani Pälli wrote:
>> On 12/08/2015 06:55 AM, Timothy Arceri wrote:
>>> On Tue, 2015-12-08 at 15:21 +1100, Timothy Arceri wrote:
>>>> On Tue, 2015-12-08 at 10:55 +1100, Timothy Arceri wrote:
>>>>> On Mon, 2015-12-07 at 11:29 +0200, Tapani Pälli wrote:
>>>>>> This will be used for validating SSO pipeline where all
>>>>>> active
>>>>>> stages
>>>>>> in linked programs should be in use when rendering.
>>>>>>
>>>>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>>>>> ---
>>>>>>    src/mesa/main/mtypes.h      |  2 ++
>>>>>>    src/mesa/main/pipelineobj.c | 39
>>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>>    2 files changed, 41 insertions(+)
>>>>>>
>>>>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>>>>> index fa7ead0..d5a22c9 100644
>>>>>> --- a/src/mesa/main/mtypes.h
>>>>>> +++ b/src/mesa/main/mtypes.h
>>>>>> @@ -2824,6 +2824,8 @@ struct gl_pipeline_object
>>>>>>       GLboolean Validated;                 /**< Pipeline
>>>>>> Validation
>>>>>> status */
>>>>>>    
>>>>>>       GLchar *InfoLog;
>>>>>> +
>>>>>> +   uint8_t ActiveStages;                /**< Stages used,
>>>>>> (glUseProgramStages) */
>>>>>>    };
>>>>>>    
>>>>>>    /**
>>>>>> diff --git a/src/mesa/main/pipelineobj.c
>>>>>> b/src/mesa/main/pipelineobj.c
>>>>>> index 6710d0d..c510ee8 100644
>>>>>> --- a/src/mesa/main/pipelineobj.c
>>>>>> +++ b/src/mesa/main/pipelineobj.c
>>>>>> @@ -218,6 +218,43 @@ _mesa_reference_pipeline_object_(struct
>>>>>> gl_context *ctx,
>>>>>>       }
>>>>>>    }
>>>>>>    
>>>>>> +static GLenum
>>>>>> +shader_bit_from_shader_stage(gl_shader_stage stage)
>>>>>> +{
>>>>>> +   switch (stage) {
>>>>>> +   case MESA_SHADER_VERTEX:
>>>>>> +      return GL_VERTEX_SHADER_BIT;
>>>>>> +   case MESA_SHADER_FRAGMENT:
>>>>>> +      return GL_FRAGMENT_SHADER_BIT;
>>>>>> +   case MESA_SHADER_GEOMETRY:
>>>>>> +      return GL_GEOMETRY_SHADER_BIT;
>>>>>> +   case MESA_SHADER_TESS_CTRL:
>>>>>> +      return GL_TESS_CONTROL_SHADER_BIT;
>>>>>> +   case MESA_SHADER_TESS_EVAL:
>>>>>> +      return GL_TESS_EVALUATION_SHADER_BIT;
>>>>>> +   case MESA_SHADER_COMPUTE:
>>>>>> +      return GL_COMPUTE_SHADER_BIT;
>>>>>> +   default:
>>>>>> +      unreachable("bad value in
>>>>>> _mesa_shader_bit_from_shader_stage()");
>>>>>> +   }
>>>>>> +}
>>>>>> +
>>>>>> +static void
>>>>>> +update_active_pipeline_stages(struct gl_pipeline_object
>>>>>> *pipe,
>>>>>> +                              struct gl_shader_program
>>>>>> *shProg,
>>>>>> +                              GLbitfield stages)
>>>>>> +{
>>>>>> +   unsigned i;
>>>>>> +   for (i = 0; i < MESA_SHADER_STAGES; i++) {
>>>>>> +      if ((stages & shader_bit_from_shader_stage(i)) != 0) {
>>>>>> +         if (shProg && shProg->ActiveStages & (1 << i))
>>>>>> +            pipe->ActiveStages |= (1 << i);
>>>>>> +         else
>>>>>> +            pipe->ActiveStages &= ~(1 << i);
>>>>>> +      }
>>>>>> +   }
>>>>> I think you could do most of the validation at this point which
>>>>> would
>>>>> reduce the amount of work required during rendering. You also
>>>>> have
>>>>> forgotten about GL_ALL_SHADER_BITS.
>>>> No you didn't I was just looking at to code for to long :P
>>>> However
>>>> unless I'm missing something I believe my other comments are
>>>> still
>>>> valid.
>>>>
>>>>> For example could you do something like this?
>>>>>
>>>>> Note: You would need to call this function after the
>>>>>    _mesa_use_shader_program() calls.
>>>>>
>>>>> unsigned i;
>>>>> unsigned j;
>>>>>
>>>>> for (i = 0; i < MESA_SHADER_STAGES; i++) {
>>>>>      if (!pipe->CurrentProgram[i]) {
>>>>>         struct gl_shader_program *shProg = pipe
>>>>> ->CurrentProgram[i];
>>>>>         for (j = 0; j < MESA_SHADER_STAGES; j++) {
>>>>>            if (!shProg->_LinkedShaders[j])
>>>>>            continue;
>>>>>
>>>>>            /* Make sure that any shader stage is also used in
>>>>> the
>>>>> pipeline */
>>>>>            if (shProg != pipe->CurrentProgram[j]) {
>>>>>               pipe->ValidProgramUse = false;
>>>>>               return;
>>>>>            }
>>>>>         }
>>>>>      }
>>>>>
>>>>>      pipe->ValidProgramUse = true;
>>>>> }
>>> Thinking about this some more you would also need to revalidate if
>>> the
>>> program has been relinked. I guess ValidProgramUse could be stored
>>> for
>>> each program object rather than just for the pipe, then you could
>>> loop
>>> over the stages in patch 3 and only do the full validation check
>>> when
>>>    ValidProgramUse is false (you would reset to false on a relink).
>> Relink is handled by having ActiveStages mask in gl_shader_program
>> (1st
>> patch) that changes during linking and then this validation will
>> fail.
> Right but I don't think using a mask is going to be able to do
> everything the validation requires. See my latest comment on patch 3.

I've experimented a bit and I believe you now :) It'll be easier to have 
a boolean in gl_shader_program. I'll implement this and try to get to 
the list today!

>> There is also case where linking fails to program which is not set
>> active, I have a patch for this that uses the mask also here, did not
>> include it yet in the series since it was not quite ready at that
>> time:
>>
>> http://cgit.freedesktop.org/~tpalli/mesa/commit/?h=fix_sso&id=cffd08b
>> 93f56244ec7f7fbbca69d7f0d6d0a6243
>>
>>> I'm also just about to add another comment to patch 3 as I think
>>> your c
>>> urrent patch will miss some use cases.
>>>
>>>>>> +}
>>>>>> +
>>>>>>    /**
>>>>>>     * Bound program to severals stages of the pipeline
>>>>>>     */
>>>>>> @@ -311,6 +348,8 @@ _mesa_UseProgramStages(GLuint pipeline,
>>>>>> GLbitfield stages, GLuint program)
>>>>>>          }
>>>>>>       }
>>>>>>    
>>>>>> +   update_active_pipeline_stages(pipe, shProg, stages);
>>>>>> +
>>>>>>       /* Enable individual stages from the program as
>>>>>> requested by
>>>>>> the
>>>>>>        * application.  If there is no shader for a requested
>>>>>> stage
>>>>>> in
>>>>>> the
>>>>>>        * program, _mesa_use_shader_program will enable fixed
>>>>>> -function
>>>>>> processing
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list