On 08/09/2013 12:33 AM, Eric Anholt wrote:
Mark Mueller <[email protected]> writes:
On Thu, Aug 8, 2013 at 2:19 PM, Eric Anholt <[email protected]> wrote:
Mark Mueller <[email protected]> writes:
Signed-off-by: Mark Mueller <[email protected]>
---
src/mesa/drivers/dri/i965/brw_draw.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index 6170d07..1b5ed55 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -367,6 +367,12 @@ static bool brw_try_draw_prims( struct gl_context
*ctx,
bool retval = true;
GLuint i;
bool fail_next = false;
+ const int estimated_max_prim_size =
+ 512 + /* batchbuffer commands */
+ ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) +
sizeof(struct gen5_sampler_default_color)))) +
+ 1024 + /* gen6 VS push constants */
+ 1024 + /* gen6 WM push constants */
+ 512; /* misc. pad */
What's the point of this change? Moving loop invariants out of loops is
something basic that your compiler does,
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.
If your compiler isn't doing that, it's a problem with your compiler,
not the code being compiled, and you need to fix that in your build
environment.
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?
for (i = 0; i < nr_prims; i++) {
const int estimated_max_prim_size =
512 + /* batchbuffer commands */
((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) +
sizeof(struct gen5_sampler_default_color)))) +
1024 + /* gen6 VS push constants */
1024 + /* gen6 WM push constants */
512; /* misc. pad */
The const keyword doesn't tell the compiler anyhing except "keep the
developer from trying to modify this", which just makes things
irritating when somebody comes along later to add something like "oh,
and on gen11 we need to reserve an extra 4k" or whatever.
It also keeps you from accidentally modifying it when you didn't mean
to. I have to side with John Carmack on this one:
"I am a full const nazi nowadays, and I chide any programmer
that doesn’t const every variable and parameter that can be."
If you need to make the constant a different size for a different
platform, use the ?: operator.
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
