On Thu, 2 May 2024, John Hubbard wrote:
> First of all, in order to build with clang at all, one must first apply
> Valentin Obst's build fix for LLVM [1]. Furthermore, for this particular
> resctrl directory, my pending fix [2] must also be applied. Once those
> fixes are in place, then when building with clang, via:
>
> make LLVM=1 -C tools/testing/selftests
>
> ..two types of warnings occur:
>
> warning: absolute value function 'abs' given an argument of type
> 'long' but has parameter of type 'int' which may cause truncation of
> value
>
> warning: taking the absolute value of unsigned type 'unsigned long'
> has no effect
>
> Fix these by:
>
> a) using labs() in place of abs(), when long integers are involved, and
>
> b) don't call labs() unnecessarily.
>
> [1]
> https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...@valentinobst.de/
> [2] https://lore.kernel.org/all/[email protected]/
>
> Signed-off-by: John Hubbard <[email protected]>
> ---
> tools/testing/selftests/resctrl/cmt_test.c | 4 ++--
> tools/testing/selftests/resctrl/mba_test.c | 2 +-
> tools/testing/selftests/resctrl/mbm_test.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cmt_test.c
> b/tools/testing/selftests/resctrl/cmt_test.c
> index a81f91222a89..05a241519ae8 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -40,11 +40,11 @@ static int show_results_info(unsigned long sum_llc_val,
> int no_of_bits,
> int ret;
>
> avg_llc_val = sum_llc_val / num_of_runs;
> - avg_diff = (long)abs(cache_span - avg_llc_val);
> + avg_diff = (long)(cache_span - avg_llc_val);
> diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
>
> ret = platform && abs((int)diff_percent) > max_diff_percent &&
> - abs(avg_diff) > max_diff;
> + labs(avg_diff) > max_diff;
>
> ksft_print_msg("%s Check cache miss rate within %lu%%\n",
> ret ? "Fail:" : "Pass:", max_diff_percent);
This seems fine but...
> diff --git a/tools/testing/selftests/resctrl/mba_test.c
> b/tools/testing/selftests/resctrl/mba_test.c
> index 7946e32e85c8..673b2bb800f7 100644
> --- a/tools/testing/selftests/resctrl/mba_test.c
> +++ b/tools/testing/selftests/resctrl/mba_test.c
> @@ -77,7 +77,7 @@ static bool show_mba_info(unsigned long *bw_imc, unsigned
> long *bw_resc)
>
> avg_bw_imc = sum_bw_imc / (NUM_OF_RUNS - 1);
> avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1);
> - 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);
>
> ksft_print_msg("%s Check MBA diff within %d%% for schemata
> %u\n",
> 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?
--
i.
