[Pixman] [PATCH v2 3/5] vmx: encapsulate the temporary variables inside the macros

[Pekka Paalanen]

On Thu, 25 Jun 2015 15:59:55 +0300
Oded Gabbay <oded.gabbay at gmail.com> wrote:

> v2: fixed whitespaces and indentation issues
> 
> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> Reviewed-by: Adam Jackson <ajax at redhat.com>
> ---
>  pixman/pixman-vmx.c | 72 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 39 insertions(+), 33 deletions(-)
> 
> diff --git a/pixman/pixman-vmx.c b/pixman/pixman-vmx.c
> index e33d9d9..f28a0fd 100644
> --- a/pixman/pixman-vmx.c
> +++ b/pixman/pixman-vmx.c
> @@ -153,13 +153,18 @@ over (vector unsigned int src,
>   */
>  
>  #define LOAD_VECTORS(dest, source)			  \
> +do {							  \
> +    vector unsigned char tmp1, tmp2;			  \
>      tmp1 = (typeof(tmp1))vec_ld (0, source);		  \
>      tmp2 = (typeof(tmp2))vec_ld (15, source);		  \
>      v ## source = (typeof(v ## source))			  \
>  	vec_perm (tmp1, tmp2, source ## _mask);		  \
> -    v ## dest = (typeof(v ## dest))vec_ld (0, dest);
> +    v ## dest = (typeof(v ## dest))vec_ld (0, dest);	  \
> +} while (0);

Here...

>  
>  #define LOAD_VECTORSC(dest, source, mask)		  \
> +do {							  \
> +    vector unsigned char tmp1, tmp2;			  \
>      tmp1 = (typeof(tmp1))vec_ld (0, source);		  \
>      tmp2 = (typeof(tmp2))vec_ld (15, source);		  \
>      v ## source = (typeof(v ## source))			  \
> @@ -168,7 +173,8 @@ over (vector unsigned int src,
>      v ## dest = (typeof(v ## dest))vec_ld (0, dest);	  \
>      tmp2 = (typeof(tmp2))vec_ld (15, mask);		  \
>      v ## mask = (typeof(v ## mask))			  \
> -	vec_perm (tmp1, tmp2, mask ## _mask);
> +    vec_perm (tmp1, tmp2, mask ## _mask);		  \
> +} while (0);

and here the final semicolon is too much. People expect to write them
when they call the macro.

But, it's not a bug really, it's just extra semicolon that can be
cleaned up later, so I won't hold this patch due that.

Another style issue is that Pixman CODING_STYLE says the braces go on
separate lines.

Is the comment about "notice you have to declare temp vars" now moot?
I also can't see tmp3 or tmp4 anywhere, so I suppose the whole comment
is just stale now?

Anyway, all that can be follow-ups.


Thanks,
pq