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

[Qemu-devel] [PATCH v2] 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]

Fix also the divisor which was taken from the wrong register
(thanks to Peter Maydell for this hint).

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

v2 adds the fix for the wrong register.

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)r2);
     /* 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)r2);
     /* calc overflow */
     env->PSW_USB_V = 0;
     if ((quotient_sign) && (abs_divisor)) {
-- 
2.1.4

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

[Bastian Koppelmann]

Hi Stefan,

On 03/07/2015 02:43 PM, 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]
>
> Fix also the divisor which was taken from the wrong register
> (thanks to Peter Maydell for this hint).
>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
> v2 adds the fix for the wrong register.
>
> Stefan
Thanks for the patch. It looks good to me so far. However I would like 
to test it on real hardware, to which I don't have access right now and 
might take about a week.

Cheers,
Bastian

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

[Bastian Koppelmann]

Hi Stefan,

On 03/07/2015 02:43 PM, 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]
>
> Fix also the divisor which was taken from the wrong register
> (thanks to Peter Maydell for this hint).
>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> [snip]

The code for dvinit.b and dvinit.h is not correctly calculating the 
overflow bit, but I can't figure out why, since the documentation of the 
real hardware seems to be wrong. I will take your patch for now, so the 
warning is gone and try to come up with a proper fix as soon as possible.

Cheers,
Bastian

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

[Bastian Koppelmann]

Hi Stefan,

On 03/23/2015 03:41 PM, Bastian Koppelmann wrote:
> [snip]
>
> The code for dvinit.b and dvinit.h is not correctly calculating the 
> overflow bit, but I can't figure out why, since the documentation of 
> the real hardware seems to be wrong. I will take your patch for now, 
> so the warning is gone and try to come up with a proper fix as soon as 
> possible.
>
> Cheers,
> Bastian
>
I have the proper fix now and will send a patch to the list in a few 
minutes. I'll keep your patch and will apply the fix on top of it.

Cheers,
Bastian