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

[Oded Gabbay]

On Thu, Jul 2, 2015 at 10:08 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 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.

I'm going to set another set of patches today, so I'll add a separate
patch that fix these issues

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

> 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?
Correct, will remove that
>
> Anyway, all that can be follow-ups.
>
>
> Thanks,
> pq