[Pixman] [PATCH 1/2] general: Ensure that iter buffers are aligned to 16 bytes

[Siarhei Siamashka]

On Wed, 28 Aug 2013 16:01:26 -0400
Søren Sandmann <sandmann at cs.au.dk> wrote:

> From: Søren Sandmann Pedersen <ssp at redhat.com>
> 
> At the moment iter buffers are only guaranteed to be aligned to an 8
> byte bit boundary. It is useful for SIMD implementations to be able to
> assume that these buffers are aligned to 16 bytes, so ensure this.
> ---
>  pixman/pixman-general.c |   22 +++++++++++++++-------
>  pixman/pixman-private.h |    3 +++
>  pixman/pixman-utils.c   |    9 +++++++++
>  3 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
> index 6310bff..60566c6 100644
> --- a/pixman/pixman-general.c
> +++ b/pixman/pixman-general.c
> @@ -114,7 +114,7 @@ general_composite_rect  (pixman_implementation_t *imp,
>                           pixman_composite_info_t *info)
>  {
>      PIXMAN_COMPOSITE_ARGS (info);
> -    uint64_t stack_scanline_buffer[(SCANLINE_BUFFER_LENGTH * 3 + 7) / 8];
> +    uint8_t stack_scanline_buffer[SCANLINE_BUFFER_LENGTH];

Don't know if this was intended, but the array on stack becomes ~3 times
smaller after this change.

Currently with SCANLINE_BUFFER_LENGTH = 8192, the buffer on stack
was used for the compositing operations having width up to
(SCANLINE_BUFFER_LENGTH / 4 bytes per pixel) = 2048. Which probably
makes sense for the people mostly having 1920x1080 or 1920x1200
monitors.

Now avoiding malloc is only going to happen when the width is less
than ~683 pixels.

>      uint8_t *scanline_buffer = (uint8_t *) stack_scanline_buffer;
>      uint8_t *src_buffer, *mask_buffer, *dest_buffer;
>      pixman_iter_t src_iter, mask_iter, dest_iter;
> @@ -137,17 +137,25 @@ general_composite_rect  (pixman_implementation_t *imp,
>  	Bpp = 16;
>      }
>  
> -    if (width * Bpp > SCANLINE_BUFFER_LENGTH)
> +#define ALIGN(addr)							\
> +    ((uint8_t *)((((unsigned long)(addr)) + 15) & (~15)))

Windows and maybe some other weird systems may have 32-bit long type
even in 64-bit mode. Using "uintptr_t" instead of "unsigned long" would
be a safer choice.

> +
> +    src_buffer = ALIGN (scanline_buffer);
> +    mask_buffer = ALIGN (src_buffer + width * Bpp);
> +    dest_buffer = ALIGN (mask_buffer + width * Bpp);
> +
> +    if (ALIGN (dest_buffer + width * Bpp) >
> +	    scanline_buffer + SCANLINE_BUFFER_LENGTH)
>      {
> -	scanline_buffer = pixman_malloc_abc (width, 3, Bpp);
> +	scanline_buffer = pixman_malloc_ab_plus_c (width, Bpp * 3, 32 * 3);
>  
>  	if (!scanline_buffer)
>  	    return;
> -    }
>  
> -    src_buffer = scanline_buffer;
> -    mask_buffer = src_buffer + width * Bpp;
> -    dest_buffer = mask_buffer + width * Bpp;
> +	src_buffer = ALIGN (scanline_buffer);
> +	mask_buffer = ALIGN (src_buffer + width * Bpp);
> +	dest_buffer = ALIGN (mask_buffer + width * Bpp);
> +    }
>  
>      if (width_flag == ITER_WIDE)
>      {
> diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
> index 9646605..0afabad 100644
> --- a/pixman/pixman-private.h
> +++ b/pixman/pixman-private.h
> @@ -787,6 +787,9 @@ pixman_malloc_ab (unsigned int n, unsigned int b);
>  void *
>  pixman_malloc_abc (unsigned int a, unsigned int b, unsigned int c);
>  
> +void *
> +pixman_malloc_ab_plus_c (unsigned int a, unsigned int b, unsigned int c);
> +
>  pixman_bool_t
>  _pixman_multiply_overflows_size (size_t a, size_t b);
>  
> diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
> index 98723a8..4a3a835 100644
> --- a/pixman/pixman-utils.c
> +++ b/pixman/pixman-utils.c
> @@ -49,6 +49,15 @@ _pixman_addition_overflows_int (unsigned int a, unsigned int b)
>  }
>  
>  void *
> +pixman_malloc_ab_plus_c (unsigned int a, unsigned int b, unsigned int c)
> +{
> +    if (!b || a >= INT32_MAX / b || (a * b) > INT32_MAX - c)
> +	return NULL;
> +
> +    return malloc (a * b + c);
> +}
> +
> +void *
>  pixman_malloc_ab (unsigned int a,
>                    unsigned int b)
>  {


-- 
Best regards,
Siarhei Siamashka