From: Reinette Chatre <reinette.chatre@intel.com>
To: "John Hubbard" <jhubbard@nvidia.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Shuah Khan <shuah@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
"Nick Desaulniers" <ndesaulniers@google.com>,
Bill Wendling <morbo@google.com>,
Justin Stitt <justinstitt@google.com>,
Fenghua Yu <fenghua.yu@intel.com>,
Valentin Obst <kernel@valentinobst.de>,
<linux-kselftest@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>, <llvm@lists.linux.dev>
Subject: Re: [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls
Date: Fri, 3 May 2024 11:37:04 -0700 [thread overview]
Message-ID: <26f3effc-6ea1-4670-a301-76df3a710fa9@intel.com> (raw)
In-Reply-To: <f908ba74-86c0-409c-854d-9da5f3917b05@nvidia.com>
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.
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;
But that still triggers:
warning: taking the absolute value of unsigned type 'unsigned long' has no effect [-Wabsolute-value]
Looks like we may need to be more explicit types and not rely on casting so much
to make the compiler happy.
Reinette
next prev parent reply other threads:[~2024-05-03 18:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 2:32 [PATCH] selftests/resctrl: fix clang build warnings related to abs(), labs() calls John Hubbard
2024-05-03 8:00 ` Ilpo Järvinen
2024-05-03 16:52 ` John Hubbard
2024-05-03 18:37 ` Reinette Chatre [this message]
2024-05-03 19:12 ` John Hubbard
2024-05-03 20:46 ` Reinette Chatre
2024-05-03 22:00 ` John Hubbard
2024-05-06 9:07 ` Ilpo Järvinen
2024-05-06 17:21 ` John Hubbard
-- strict thread matches above, loose matches on Subject: below --
2024-05-08 19:02 John Hubbard
2024-05-08 19:39 ` John Hubbard
2024-05-10 9:06 ` Ilpo Järvinen
2024-05-10 18:01 ` John Hubbard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=26f3effc-6ea1-4670-a301-76df3a710fa9@intel.com \
--to=reinette.chatre@intel.com \
--cc=fenghua.yu@intel.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jhubbard@nvidia.com \
--cc=justinstitt@google.com \
--cc=kernel@valentinobst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=morbo@google.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).