[Mesa-dev] [PATCH 4/8] st/mesa: determine states used or affected by shaders at compile time

[Marek Olšák]

On Wed, Aug 10, 2016 at 4:34 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 10.08.2016 13:05, Marek Olšák wrote:
>>
>> On Tue, Aug 9, 2016 at 12:56 PM, Nicolai Hähnle <nhaehnle at gmail.com>
>> wrote:
>>>
>>> On 07.08.2016 03:12, Marek Olšák wrote:
>>>>
>>>>
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> At compile time, each shader determines which ST_NEW flags should be set
>>>> at shader bind time.
>>>>
>>>> This just sets the new field for all shaders. The next commit will use
>>>> it.
>>>> ---
>>>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 175
>>>> ++++++++++++++++++++++++++++-
>>>>  src/mesa/state_tracker/st_program.c        |  37 +++++-
>>>>  src/mesa/state_tracker/st_program.h        |   6 +
>>>>  3 files changed, 215 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> index 362559f..fd14766 100644
>>>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>>>> @@ -6666,31 +6666,202 @@ get_mesa_program_tgsi(struct gl_context *ctx,
>>>>
>>>>  static struct gl_program *
>>>>  get_mesa_program(struct gl_context *ctx,
>>>>                   struct gl_shader_program *shader_program,
>>>>                   struct gl_linked_shader *shader)
>>>>  {
>>>>     struct pipe_screen *pscreen = ctx->st->pipe->screen;
>>>>     unsigned ptarget = st_shader_stage_to_ptarget(shader->Stage);
>>>>     enum pipe_shader_ir preferred_ir = (enum pipe_shader_ir)
>>>>        pscreen->get_shader_param(pscreen, ptarget,
>>>> PIPE_SHADER_CAP_PREFERRED_IR);
>>>> +   struct gl_program *prog = NULL;
>>>> +
>>>>     if (preferred_ir == PIPE_SHADER_IR_NIR) {
>>>>        /* TODO only for GLSL VS/FS for now: */
>>>>        switch (shader->Stage) {
>>>>        case MESA_SHADER_VERTEX:
>>>>        case MESA_SHADER_FRAGMENT:
>>>> -         return st_nir_get_mesa_program(ctx, shader_program, shader);
>>>> +         prog = st_nir_get_mesa_program(ctx, shader_program, shader);
>>>>        default:
>>>>           break;
>>>>        }
>>>> +   } else {
>>>> +      prog = get_mesa_program_tgsi(ctx, shader_program, shader);
>>>> +   }
>>>> +
>>>> +   if (prog) {
>>>> +      uint64_t *states;
>>>> +
>>>> +      /* This determines which states will be updated when the shader
>>>> is
>>>> +       * bound.
>>>> +       */
>>>> +      switch (shader->Stage) {
>>>> +      case MESA_SHADER_VERTEX:
>>>> +         states = &((struct st_vertex_program*)prog)->affected_states;
>>>> +
>>>> +         *states = ST_NEW_VS_STATE |
>>>> +                   ST_NEW_RASTERIZER |
>>>> +                   ST_NEW_VERTEX_ARRAYS;
>>>> +
>>>> +         if (prog->Parameters->NumParameters)
>>>> +            *states |= ST_NEW_VS_CONSTANTS;
>>>> +
>>>> +         if (shader->num_samplers)
>>>> +            *states |= ST_NEW_VS_SAMPLER_VIEWS |
>>>> +                       ST_NEW_RENDER_SAMPLERS;
>>>> +
>>>> +         if (shader->NumImages)
>>>> +            *states |= ST_NEW_VS_IMAGES;
>>>> +
>>>> +         if (shader->NumUniformBlocks)
>>>> +            *states |= ST_NEW_VS_UBOS;
>>>> +
>>>> +         if (shader->NumShaderStorageBlocks)
>>>> +            *states |= ST_NEW_VS_SSBOS;
>>>> +
>>>> +         if (shader->NumAtomicBuffers)
>>>> +            *states |= ST_NEW_VS_ATOMICS;
>>>
>>>
>>>
>>> I'm not overly fond of the code duplication here. Perhaps these could all
>>> be
>>> expressed relative to a stage-specific base flag?
>>
>>
>> Not really. It shouldn't rely on the order the flags are declared. I
>> can't make it better than this:
>
>
> I don't know why we can't just say we should be able to rely on the order,
> but the alternative below is indeed not really any better.

We could rely on the order, but as with system value enumerations,
we'd need a lot of assertions to prevent a breakage if somebody
changes the order and we don't notice.

I'll still amend the diff, because it makes the code a little shorter.

Marek