On Tue, 22 Sep 2015 12:43:25 +0100
Ben Avison <[email protected]> wrote:

> Each of the aligns can only add a maximum of 15 bytes to the space
> requirement. This permits some edge cases to use the stack buffer where
> previously it would have deduced that a heap buffer was required.
> ---
> 
> This is an update of my previous patch (now posted over a year ago):
> https://patchwork.freedesktop.org/patch/49898/
> 
> which conflicts with the patch just pushed:
> http://patchwork.freedesktop.org/patch/60052/
> 
> Since this piece of code is fresh in people's minds, this might be a good
> time to get this one pushed as well.
> 
> Note that the scope of this change has been reduced: while the old code
> added space to align the end address to a cacheline boundary (which as I
> pointed out, was unnecessary), the new version works using buffer lengths
> only.
> 
> As a discussion point, wouldn't it be better for the ALIGN macro to
> assume 32-byte cache lines? Both 16-byte and 32-byte cachelines are
> currently in common use, but by aligning the buffers to 32-byte addresses
> we would simultaneously achieve 16-byte alignment.
> 
>  pixman/pixman-general.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
> index fa88463..6141cb0 100644
> --- a/pixman/pixman-general.c
> +++ b/pixman/pixman-general.c
> @@ -158,9 +158,9 @@ general_composite_rect  (pixman_implementation_t *imp,
>      if (width <= 0 || _pixman_multiply_overflows_int (width, Bpp * 3))
>       return;
>  
> -    if (width * Bpp * 3 > sizeof (stack_scanline_buffer) - 32 * 3)
> +    if (width * Bpp * 3 > sizeof (stack_scanline_buffer) - 15 * 3)
>      {
> -     scanline_buffer = pixman_malloc_ab_plus_c (width, Bpp * 3, 32 * 3);
> +     scanline_buffer = pixman_malloc_ab_plus_c (width, Bpp * 3, 15 * 3);
>  
>       if (!scanline_buffer)
>           return;

Reviewed-by: Pekka Paalanen <[email protected]>

I think Ben's explanation as seen in
https://patchwork.freedesktop.org/patch/49898/
covers all Søren's concerns (it quotes everything Søren said about the
patch), and I see no reason to reject this.

I'll push this on Friday if no-one objects.


Thanks,
pq

Attachment: pgpL74xwNlDmG.pgp
Description: OpenPGP digital signature

_______________________________________________
Pixman mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pixman

Reply via email to