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

[Paul Berry]

On 20 August 2013 01:20, Kenneth Graunke <kenneth at whitecape.org> wrote:

> 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 <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.
>

Ouch!  That detail completely eluded me.  Thanks for checking :)


>
> I've gone ahead and pushed a corrected version of this patch so they match.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130820/2655446d/attachment.html>