On 5/6/24 2:07 AM, Ilpo Järvinen wrote:
On Fri, 3 May 2024, John Hubbard wrote:
On 5/3/24 11:37 AM, Reinette Chatre wrote:
On 5/3/2024 9:52 AM, John Hubbard wrote:
On 5/3/24 1:00 AM, Ilpo Järvinen wrote:
On Thu, 2 May 2024, John Hubbard wrote:
...
diff --git a/tools/testing/selftests/resctrl/mbm_test.c
b/tools/testing/selftests/resctrl/mbm_test.c
index d67ffa3ec63a..c873793d016d 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -33,7 +33,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long
*bw_resc, size_t span)
avg_bw_imc = sum_bw_imc / 4;
avg_bw_resc = sum_bw_resc / 4;
- avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
+ avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
avg_diff_per = (int)(avg_diff * 100);
ret = avg_diff_per > MAX_DIFF_PERCENT;
But how are these two cases same after your change when you ended up
removing taking the absolute value entirely?
All of the arguments are unsigned integers, so all arithmetic results
are interpreted as unsigned, so taking the absolute value of that is
always a no-op.
(I see there's a better patch posted already but since there are a few
incorrect claims in this discussion, I'll do for the record type of
reply.)
This discussion now went to a tangent about the warning. My main point is
that logic is not correct after removing labs().
I also disagree with the claim that using labs() on unsigned value is
no-op because labs() takes long so unsigned is just forced into signed
when calling which is why the warning triggers but it's very misleading
warning (see below).
Yes you are correct.
It does not seem as though clang can see when values have been casted.
I tried to do so explicitly with a:
avg_diff = labs((long)avg_bw_resc - avg_bw_imc) / (float)avg_bw_imc;
The subtraction result will get promoted to an unsigned long, before being
passed into labs(3).
But that still triggers:
warning: taking the absolute value of unsigned type 'unsigned long' has no
effect [-Wabsolute-value]
As expected, yes.
That error message isn't factually correct:
unsigned long a = LONG_MAX;
long b;
a += 2;
b = (long)a;
printf("%llu %lli %lli\n", a, b, labs(a));
Prints (at least when built with gcc):
9223372036854775809 -9223372036854775807 9223372036854775807
labs(LONG_MAX + 1) won't work though since it's not positively presentable
with long and the value is left untouched.
Thanks for setting the detailed record straight! :)
thanks,
--
John Hubbard
NVIDIA
