diff mbox series

[v4,2/8] target/mips: Workaround for checkpatch.pl hanging on msa_helper.c

Message ID 1530877732-26557-3-git-send-email-aleksandar.markovic@rt-rk.com
State New
Headers show
Series Maintenance and misc fixes and improvements | expand

Commit Message

Aleksandar Markovic July 6, 2018, 11:48 a.m. UTC
From: Aleksandar Markovic <amarkovic@wavecomp.com>

If checkpatch.pl is applied (using switch "-f") on file
target/mips/msa_helper.c, it will hang.

This is a workaround by correcting the source file. The workaround is
found by partial deleting and undeleting of the code in msa_helper.c
in binary search fashion.

The bug (for checkpatch.pl) is already reported to the qemu-devel list.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/msa_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Aleksandar Markovic July 6, 2018, 3:38 p.m. UTC | #1
Hi, Paolo,

It was an incredibly fast fix! :)

I already confirmed that the fix fixes the problem on msa_helper.c. I would nevertheless like to have this workaround applied. Can you perhaps give it "Reviewed-by"?

Regards,
Aleksandar


> Subject: [PATCH v4 2/8] target/mips: Workaround for checkpatch.pl hanging on > msa_helper.c
>
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> If checkpatch.pl is applied (using switch "-f") on file
> target/mips/msa_helper.c, it will hang.
>
> This is a workaround by correcting the source file. The workaround is
> found by partial deleting and undeleting of the code in msa_helper.c
> in binary search fashion.
>
> The bug (for checkpatch.pl) is already reported to the qemu-devel list.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/msa_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
> index c74e3cd..1691b70 100644
> --- a/target/mips/msa_helper.c
> +++ b/target/mips/msa_helper.c
> @@ -2750,8 +2750,8 @@ void helper_msa_ftq_df(CPUMIPSState *env, uint32_t df, > uint32_t wd,
>
>  #define FMAXMIN_A(F, G, X, _S, _T, BITS, STATUS)                    \
>      do {                                                            \
> -        uint## BITS ##_t S = _S, T = _T;                            \
> -        uint## BITS ##_t as, at, xs, xt, xd;                        \
> +        uint## BITS ## _t S = _S, T = _T;                           \
> +        uint## BITS ## _t as, at, xs, xt, xd;                       \
>          if (NUMBER_QNAN_PAIR(S, T, BITS, STATUS)) {                 \
>              T = S;                                                  \
>          }                                                           \
> --
> 2.7.4
Philippe Mathieu-Daudé July 6, 2018, 4:52 p.m. UTC | #2
Hi Aleksandar,

On 07/06/2018 12:38 PM, Aleksandar Markovic wrote:
> Hi, Paolo,
> 
> It was an incredibly fast fix! :)
> 
> I already confirmed that the fix fixes the problem on msa_helper.c. I would nevertheless like to have this workaround applied. Can you perhaps give it "Reviewed-by"?
> 
> Regards,
> Aleksandar
> 
> 
>> Subject: [PATCH v4 2/8] target/mips: Workaround for checkpatch.pl hanging on > msa_helper.c
>>
>> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>>
>> If checkpatch.pl is applied (using switch "-f") on file
>> target/mips/msa_helper.c, it will hang.
>>
>> This is a workaround by correcting the source file. The workaround is
>> found by partial deleting and undeleting of the code in msa_helper.c
>> in binary search fashion.
>>
>> The bug (for checkpatch.pl) is already reported to the qemu-devel list.
>>
>> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>> ---
>>  target/mips/msa_helper.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
>> index c74e3cd..1691b70 100644
>> --- a/target/mips/msa_helper.c
>> +++ b/target/mips/msa_helper.c
>> @@ -2750,8 +2750,8 @@ void helper_msa_ftq_df(CPUMIPSState *env, uint32_t df, > uint32_t wd,
>>
>>  #define FMAXMIN_A(F, G, X, _S, _T, BITS, STATUS)                    \
>>      do {                                                            \
>> -        uint## BITS ##_t S = _S, T = _T;                            \
>> -        uint## BITS ##_t as, at, xs, xt, xd;                        \
>> +        uint## BITS ## _t S = _S, T = _T;                           \
>> +        uint## BITS ## _t as, at, xs, xt, xd;                       \

I'm not sure it's worth having this but you are the maintainer so your
choice :) What is unclear to me is, while changing this, why only fix
the suffix and not also the prefix? That is:

            uint<space>## BITS ##<space>_t S = _S, T = _T;

>>          if (NUMBER_QNAN_PAIR(S, T, BITS, STATUS)) {                 \
>>              T = S;                                                  \
>>          }                                                           \
>> --
>> 2.7.4
Aleksandar Markovic July 6, 2018, 5:11 p.m. UTC | #3
> I'm not sure it's worth having this but you are the maintainer so your
> choice :)

If someone in future peruses an older version of this file (let's say, while working on an older version of QEMU), and comes across this problem with checkpatch.pl, by checking the full history of the file, they would be able to see what the problem is. Without this patch, they wouldn't.

> What is unclear to me is, while changing this, why only fix
> the suffix and not also the prefix? That is:
>
>            uint<space>## BITS ##<space>_t S = _S, T = _T;

It wouldn't be clear what characters caused the problem (or triggered the bug, if you wish).

Regards,
Aleksandar
diff mbox series

Patch

diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index c74e3cd..1691b70 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -2750,8 +2750,8 @@  void helper_msa_ftq_df(CPUMIPSState *env, uint32_t df, uint32_t wd,
 
 #define FMAXMIN_A(F, G, X, _S, _T, BITS, STATUS)                    \
     do {                                                            \
-        uint## BITS ##_t S = _S, T = _T;                            \
-        uint## BITS ##_t as, at, xs, xt, xd;                        \
+        uint## BITS ## _t S = _S, T = _T;                           \
+        uint## BITS ## _t as, at, xs, xt, xd;                       \
         if (NUMBER_QNAN_PAIR(S, T, BITS, STATUS)) {                 \
             T = S;                                                  \
         }                                                           \