[Qemu-devel] [PATCH] target-tricore: Fix two helper functions (clang warnings)

[Qemu-devel] [PATCH] target-tricore: Fix two helper functions (clang warnings)

[Stefan Weil]

clang report:

target-tricore/op_helper.c:1247:24: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1248:25: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1249:19: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1297:24: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1298:25: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1299:19: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]

Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

I'm not sure whether this is the correct fix, so please review carefully.

Thanks,
Stefan

 target-tricore/op_helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index ed26b30..52ad80b 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -1244,9 +1244,9 @@ uint64_t helper_dvinit_b_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
         quotient_sign = 1;
     }
 
-    abs_sig_dividend = abs(r1) >> 7;
-    abs_base_dividend = abs(r1) & 0x7f;
-    abs_divisor = abs(r1);
+    abs_sig_dividend = abs((int32_t)r1) >> 7;
+    abs_base_dividend = abs((int32_t)r1) & 0x7f;
+    abs_divisor = abs((int32_t)r1);
     /* calc overflow */
     env->PSW_USB_V = 0;
     if ((quotient_sign) && (abs_divisor)) {
@@ -1294,9 +1294,9 @@ uint64_t helper_dvinit_h_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
         quotient_sign = 1;
     }
 
-    abs_sig_dividend = abs(r1) >> 7;
-    abs_base_dividend = abs(r1) & 0x7f;
-    abs_divisor = abs(r1);
+    abs_sig_dividend = abs((int32_t)r1) >> 7;
+    abs_base_dividend = abs((int32_t)r1) & 0x7f;
+    abs_divisor = abs((int32_t)r1);
     /* calc overflow */
     env->PSW_USB_V = 0;
     if ((quotient_sign) && (abs_divisor)) {
-- 
2.1.4

Re: [Qemu-devel] [PATCH] target-tricore: Fix two helper functions (clang warnings)

[Bastian Koppelmann]

Hi Stefan,

On 03/07/2015 09:33 AM, Stefan Weil wrote:
> clang report:
>
> target-tricore/op_helper.c:1247:24: warning:
>    taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
>    has no effect [-Wabsolute-value]
> target-tricore/op_helper.c:1248:25: warning:
>    taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
>    has no effect [-Wabsolute-value]
> target-tricore/op_helper.c:1249:19: warning:
>    taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
>    has no effect [-Wabsolute-value]
> target-tricore/op_helper.c:1297:24: warning:
>    taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
>    has no effect [-Wabsolute-value]
> target-tricore/op_helper.c:1298:25: warning:
>    taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
>    has no effect [-Wabsolute-value]
> target-tricore/op_helper.c:1299:19: warning:
>    taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
>    has no effect [-Wabsolute-value]
>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
Good catch. I'm really confused, that this was not breaking my test 
suit. Your fix looks good to me at first sight, but I'll look into the 
problem.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH] target-tricore: Fix two helper functions (clang warnings)

[Peter Maydell]

On 7 March 2015 at 18:33, Stefan Weil <sw@weilnetz.de> wrote:
> diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
> index ed26b30..52ad80b 100644
> --- a/target-tricore/op_helper.c
> +++ b/target-tricore/op_helper.c
> @@ -1244,9 +1244,9 @@ uint64_t helper_dvinit_b_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
>          quotient_sign = 1;
>      }
>
> -    abs_sig_dividend = abs(r1) >> 7;
> -    abs_base_dividend = abs(r1) & 0x7f;
> -    abs_divisor = abs(r1);
> +    abs_sig_dividend = abs((int32_t)r1) >> 7;
> +    abs_base_dividend = abs((int32_t)r1) & 0x7f;
> +    abs_divisor = abs((int32_t)r1);

Also, surely some of these should be using r2, not r1?
At the moment we seem to use r1 for both dividend and
divisor, which does not look right at all...

-- PMM

Re: [Qemu-devel] [PATCH] target-tricore: Fix two helper functions (clang warnings)

[Bastian Koppelmann]

On 03/07/2015 12:34 PM, Peter Maydell wrote:
> On 7 March 2015 at 18:33, Stefan Weil <sw@weilnetz.de> wrote:
>> diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
>> index ed26b30..52ad80b 100644
>> --- a/target-tricore/op_helper.c
>> +++ b/target-tricore/op_helper.c
>> @@ -1244,9 +1244,9 @@ uint64_t helper_dvinit_b_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
>>           quotient_sign = 1;
>>       }
>>
>> -    abs_sig_dividend = abs(r1) >> 7;
>> -    abs_base_dividend = abs(r1) & 0x7f;
>> -    abs_divisor = abs(r1);
>> +    abs_sig_dividend = abs((int32_t)r1) >> 7;
>> +    abs_base_dividend = abs((int32_t)r1) & 0x7f;
>> +    abs_divisor = abs((int32_t)r1);
> Also, surely some of these should be using r2, not r1?
> At the moment we seem to use r1 for both dividend and
> divisor, which does not look right at all...
>
> -- PMM
Thats correct, it should be:

abs_divisor = abs((int32_t)r2);

My test suit did not get this, because I was only testing with a ISA 
version 1.3.1 cpu and this is 1.3 exclusive. Thanks for finding those.

Cheers,
Bastian