[Mesa-dev] [PATCH 1/6] mesa, gallium: renumber shader indices according to their placement in pipeline

[Brian Paul]

On 06/28/2013 08:53 AM, Jose Fonseca wrote:
>
>
> ----- Original Message -----
>> See my explanation in mtypes.h.
>> ---
>>   src/gallium/include/pipe/p_defines.h       |    7 ++++---
>>   src/glsl/linker.cpp                        |   16 ++++++++--------
>>   src/mesa/drivers/dri/i965/brw_shader.cpp   |    8 ++------
>>   src/mesa/main/mtypes.h                     |    8 ++++++--
>>   src/mesa/main/shaderobj.h                  |    4 ++--
>>   src/mesa/main/uniform_query.cpp            |    2 +-
>>   src/mesa/program/ir_to_mesa.cpp            |   10 +++-------
>>   src/mesa/program/program.h                 |    2 +-
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   10 +++-------
>>   9 files changed, 30 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/gallium/include/pipe/p_defines.h
>> b/src/gallium/include/pipe/p_defines.h
>> index 8af1a84..216cd2f 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -352,11 +352,12 @@ enum pipe_flush_flags {
>>
>>
>>   /**
>> - * Shaders
>> + * Shaders.
>> + * These must have the same values as Mesa's MESA_SHADER_*.
>
> Sorry for not noticing this earlier -- I haven't been able to keep up with email traffic lately.
>
> I'm afraid I can't agree with this.  Gallium needs to be API agnostic -- it doesn't make sense to have gallium go at the whims of particular state tracker implementation details.
>
> There is a lot of code out there that relies on this ordering. And unfortunately reordering will cause it to fail, often in a silent manner (with no compiler errors or warnings).
>
> ----- Original Message -----
>> The renumbering only makes sense for the GLSL linker and the only
>> reason for doing that in gallium too is that PIPE_SHADER_x must be
>> equal to MESA_SHADER_x.
>
> This is an implementation detail.
>
> PIPE_SHADER_x may have been paired with MESA_SHADER_x till now as convenience, but now they are what they are.


I took a quick look at the state tracker.  AFAICT, there's only one 
place where we obviously depend on MESA_SHADER_x == PIPE_SHADER_x, at 
st_extensions.c:152

The st_atom_shader/constbuf, etc. code looks OK.

I think we could remove the MESA_SHADER_x == PIPE_SHADER_x assumption 
without too much trouble.

-Brian