| Message ID | 1425739412-8144-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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)) {
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(-)