[Mesa-dev] [PATCH 09/10] i965: Shorten sampler loops in precompile key setup.

[Kenneth Graunke]

On 08/18/2013 05:58 PM, Kenneth Graunke wrote:
> On 08/15/2013 10:52 PM, Paul Berry wrote:
>> On 14 August 2013 18:55, Kenneth Graunke <kenneth at whitecape.org
>> <mailto:kenneth at whitecape.org>> wrote:
>>
>>     Now that we have the number of samplers available, we don't need to
>>     iterate over all 16.  This should be particularly helpful for vertex
>>     shaders.
>>
>>     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
>>     <mailto:kenneth at whitecape.org>>
>>     ---
>>       src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
>>       src/mesa/drivers/dri/i965/brw_vs.c   | 2 +-
>>       2 files changed, 2 insertions(+), 2 deletions(-)
>>
>>     diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>     b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>     index 69e544a..27263fd 100644
>>     --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>     +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>     @@ -3143,7 +3143,7 @@ brw_fs_precompile(struct gl_context *ctx,
>>     struct gl_shader_program *prog)
>>
>>          key.clamp_fragment_color = ctx->API == API_OPENGL_COMPAT;
>>
>>     -   for (int i = 0; i < MAX_SAMPLERS; i++) {
>>     +   for (unsigned i = 0; i < brw->wm.sampler_count; i++) {
>>
>>
>> Precompile is called during link, before the program is made current.
>> So brw->wm is some other program.  I think you need to do this instead:
>> _mesa_fls(fp->Base.SamplersUsed).
>>
>>             if (fp->Base.ShadowSamplers & (1 << i)) {
>>                /* Assume DEPTH_TEXTURE_MODE is the default: X, X, X, 1 */
>>                key.tex.swizzles[i] =
>>     diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
>>     b/src/mesa/drivers/dri/i965/brw_vs.c
>>     index 5b8173d..7df93c2 100644
>>     --- a/src/mesa/drivers/dri/i965/brw_vs.c
>>     +++ b/src/mesa/drivers/dri/i965/brw_vs.c
>>     @@ -544,7 +544,7 @@ brw_vs_precompile(struct gl_context *ctx, struct
>>     gl_shader_program *prog)
>>          key.base.program_string_id = bvp->id;
>>          key.base.clamp_vertex_color = ctx->API == API_OPENGL_COMPAT;
>>
>>     -   for (int i = 0; i < MAX_SAMPLERS; i++) {
>>     +   for (unsigned i = 0; i < brw->vs.sampler_count; i++) {
>>
>>
>> Similarly, this should be _mesa_fls(vp->Base.SamplersUsed).
>>
>> With those fixes, this patch is:
>>
>> Reviewed-by: Paul Berry <stereotype441 at gmail.com
>> <mailto:stereotype441 at gmail.com>>
>
> Yikes, you're right.  I think I'll just drop this patch - it's not worth
> the complexity, especially since I doubt there's any measureable benefit.
>
> --Ken

I take that back.  Both patches need to land, or neither...otherwise 
precompile sets everything to XYZW, but the actual key setup sets them 
to 0, and we get pointless recompiles.

I've gone ahead and pushed a corrected version of this patch so they match.