[Mesa-dev] [PATCH 2/2] i965: add runtime check for SSSE3 rgba8_copy

[Chad Versace]

On Thu 06 Nov 2014, Timothy Arceri wrote:
>Also cleans up some if statements in the *faster functions.

I have comments about the cleanup below.

>diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
>index cb5738a..0deeb75 100644
>--- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
>+++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c

/**
 * Copy texture data from linear to X tile layout, faster.
 *
 * Same as \ref xtile_copy but faster, because it passes constant parameters
 * for common cases, allowing the compiler to inline code optimized for those
 * cases.
 *
 * \copydoc tile_copy_fn
 */
static FLATTEN void
xtile_copy_faster(...)

>@@ -352,19 +316,8 @@ xtile_copy_faster(uint32_t x0, uint32_t x1, uint32_t x2, uint32_t x3,
>                   mem_copy_fn mem_copy)


> {
>    if (x0 == 0 && x3 == xtile_width && y0 == 0 && y1 == xtile_height) {
>-      if (mem_copy == memcpy)
>-         return xtile_copy(0, 0, xtile_width, xtile_width, 0, xtile_height,
>-                           dst, src, src_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>-         return xtile_copy(0, 0, xtile_width, xtile_width, 0, xtile_height,
>-                           dst, src, src_pitch, swizzle_bit, rgba8_copy);
>-   } else {
>-      if (mem_copy == memcpy)
>-         return xtile_copy(x0, x1, x2, x3, y0, y1,
>-                           dst, src, src_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>-         return xtile_copy(x0, x1, x2, x3, y0, y1,
>-                           dst, src, src_pitch, swizzle_bit, rgba8_copy);
>+      return xtile_copy(0, 0, xtile_width, xtile_width, 0, xtile_height,
>+                        dst, src, src_pitch, swizzle_bit, mem_copy);
>    }
>    xtile_copy(x0, x1, x2, x3, y0, y1,
>               dst, src, src_pitch, swizzle_bit, mem_copy);

The "cleanup" of this if tree concerns me. Accoring the function
comment, the original author of this function, fjhenigman, clearly 
created the weird 'if' tree with the intentation that the compiler would 
"inline code optimized for those cases".

Without one of the following, I object to this cleanup:
    - Frank's approval, or
    - Proof that gcc never does the desired optimizations, or
    - Proof that this change does not harm's Chrome's texture upload 
    performance.