<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 8, 2013 at 2:19 PM, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">Mark Mueller <<a href="mailto:markkmueller@gmail.com">markkmueller@gmail.com</a>> writes:<br>

> Signed-off-by: Mark Mueller <<a href="mailto:MarkKMueller@gmail.com">MarkKMueller@gmail.com</a>><br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_draw.c | 16 ++++++----------<br>
>  1 file changed, 6 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c<br>
> index 6170d07..1b5ed55 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_draw.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c<br>
> @@ -367,6 +367,12 @@ static bool brw_try_draw_prims( struct gl_context *ctx,<br>
>     bool retval = true;<br>
>     GLuint i;<br>
>     bool fail_next = false;<br>
> +   const int estimated_max_prim_size =<br>
> +           512 + /* batchbuffer commands */<br>
> +           ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color)))) +<br>
> +           1024 + /* gen6 VS push constants */<br>
> +           1024 + /* gen6 WM push constants */<br>
> +           512; /* misc. pad */<br>
<br>
</div>What's the point of this change?  Moving loop invariants out of loops is<br>
something basic that your compiler does,</blockquote><div><br></div><div>Is that universally true for the code as it looked originally (see below)? I've worked on embedded Atom and other systems with heavily dumbed down gcc or other cross compilers. For instance there is a good chance that the compilers from vehicle infotainment systems that I've worked on recently would generate assembly for each line of code below inside the loop.</div>
<div><br></div><div>original code:</div><pre style="padding:0px;margin-top:0px;margin-bottom:0px;color:rgb(0,0,0);font-size:13px"><code>      int estimated_max_prim_size;

</code></pre><pre style="padding:0px;margin-top:0px;margin-bottom:0px;color:rgb(0,0,0);font-size:13px"><code>      estimated_max_prim_size = 512; /* batchbuffer commands */
      estimated_max_prim_size += (BRW_MAX_TEX_UNIT *
                                  (sizeof(struct brw_sampler_state) +
                                   sizeof(struct gen5_sampler_default_color)));
      estimated_max_prim_size += 1024; /* gen6 VS push constants */
      estimated_max_prim_size += 1024; /* gen6 WM push constants */
      estimated_max_prim_size += 512; /* misc. pad */
.</code></pre><div>.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> and you're moving the<br>

definition farther from its use.<br></blockquote><div><br></div><div>Sure, yet it's in the company of fail_next with a similar problem. What about keeping the definition inside the for loop but adding the const keyword and adding all of the immediates as one operation?</div>
<div><br></div><pre style="font-size:13px;padding:0px;margin-top:0px;margin-bottom:0px"><code><font color="#000000">   for (i = 0; i < nr_prims; i++) {
</font></code></pre><div><font color="#000000">         const int estimated_max_prim_size =<br>                 512 + /* batchbuffer commands */<br>                 ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color)))) +<br>
                 1024 + /* gen6 VS push constants */<br>                 1024 + /* gen6 WM push constants */<br>                 512; /* misc. pad */<br></font></div></div><br></div><div class="gmail_extra">I'm also fine with dropping the whole thing and moving on as the real point of this was to calibrate myself to the contribution process. It's been good for that.</div>
<div class="gmail_extra"><br></div></div>