[Mesa-dev] [PATCH 8/8] mesa: Micro-optimize _mesa_is_valid_prim_mode
Kenneth Graunke
kenneth at whitecape.org
Sun Dec 21 23:33:50 PST 2014
On Friday, December 19, 2014 02:20:59 PM Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> You would not believe the mess GCC 4.8.3 generated for the old
> switch-statement.
>
> On Bay Trail-D using Fedora 20 compile flags (-m64 -O2 -mtune=generic
> for 64-bit and -m32 -march=i686 -mtune=atom for 32-bit), affects
> Gl32Batch7:
>
> 32-bit: Difference at 95.0% confidence -0.37374% +/- 0.184057% (n=40)
> 64-bit: Difference at 95.0% confidence 0.966722% +/- 0.338442% (n=40)
>
> The regression on 32-bit is odd. Callgrind says the caller,
> _mesa_is_valid_prim_mode is faster. Before it says 2,293,760
> cycles, and after it says 917,504.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
> src/mesa/main/api_validate.c | 30 ++++++++++++------------------
> 1 file changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index b882f0e..9c2e29e 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -113,27 +113,21 @@ check_valid_to_render(struct gl_context *ctx, const char *function)
> bool
> _mesa_is_valid_prim_mode(struct gl_context *ctx, GLenum mode)
> {
> - switch (mode) {
> - case GL_POINTS:
> - case GL_LINES:
> - case GL_LINE_LOOP:
> - case GL_LINE_STRIP:
> - case GL_TRIANGLES:
> - case GL_TRIANGLE_STRIP:
> - case GL_TRIANGLE_FAN:
> + /* The overwhelmingly common case is (mode <= GL_TRIANGLE_FAN). Test that
> + * first and exit. You would think that a switch-statement would be the
> + * right approach, but at least GCC 4.7.2 generates some pretty dire code
Perhaps update this to 4.8.3 to match your commit message?
> + * for the common case.
> + */
> + if (likely(mode <= GL_TRIANGLE_FAN))
I'm not sure if likely() is a good idea. My understanding is the penalty for
getting it wrong is pretty hefty, and the other modes do happen.
Getting rid of the switch statement makes a lot of sense. I like the new
approach.
With likely removed, this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> return true;
> - case GL_QUADS:
> - case GL_QUAD_STRIP:
> - case GL_POLYGON:
> +
> + if (mode <= GL_POLYGON)
> return (ctx->API == API_OPENGL_COMPAT);
> - case GL_LINES_ADJACENCY:
> - case GL_LINE_STRIP_ADJACENCY:
> - case GL_TRIANGLES_ADJACENCY:
> - case GL_TRIANGLE_STRIP_ADJACENCY:
> +
> + if (mode <= GL_TRIANGLE_STRIP_ADJACENCY)
> return _mesa_has_geometry_shaders(ctx);
> - default:
> - return false;
> - }
> +
> + return false;
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141221/a73edcbf/attachment-0001.sig>
More information about the mesa-dev
mailing list