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

[Kenneth Graunke]

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