lkml.org 
[lkml]   [2021]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] lib: zstd: Add cast to silence clang's -Wbitwise-instead-of-logical
Date


> On Oct 21, 2021, at 1:23 PM, Nathan Chancellor <nathan@kernel.org> wrote:
>
> A new warning in clang warns that there is an instance where boolean
> expressions are being used with bitwise operators instead of logical
> ones:
>
> lib/zstd/decompress/huf_decompress.c:890:25: warning: use of bitwise '&' with boolean operands [-Wbitwise-instead-of-logical]
> (BIT_reloadDStreamFast(&bitD1) == BIT_DStream_unfinished)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> zstd does this frequently to help with performance, as logical operators
> have branches whereas bitwise ones do not.
>
> To fix this warning in other cases, the expressions were placed on
> separate lines with the '&=' operator; however, this particular instance
> was moved away from that so that it could be surrounded by LIKELY, which
> is a macro for __builtin_expect(), to help with a performance
> regression, according to upstream zstd pull #1973.
>
> Aside from switching to logical operators, which is likely undesirable
> in this instance, or disabling the warning outright, the solution is
> casting one of the expressions to an integer type to make it clear to
> clang that the author knows what they are doing. Add a cast to U32 to
> silence the warning. The first U32 cast is to silence an instance of
> -Wshorten-64-to-32 because __builtin_expect() returns long so it cannot
> be moved.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1486
> Link: https://github.com/facebook/zstd/pull/1973
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the fix!

I’ll apply this patch to my linux-next tree and port it to upstream zstd.

Best,
Nick Terrell

> ---
> lib/zstd/decompress/huf_decompress.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/zstd/decompress/huf_decompress.c b/lib/zstd/decompress/huf_decompress.c
> index 05570ed5f8be..5105e59ac04a 100644
> --- a/lib/zstd/decompress/huf_decompress.c
> +++ b/lib/zstd/decompress/huf_decompress.c
> @@ -886,7 +886,7 @@ HUF_decompress4X2_usingDTable_internal_body(
> HUF_DECODE_SYMBOLX2_0(op2, &bitD2);
> HUF_DECODE_SYMBOLX2_0(op3, &bitD3);
> HUF_DECODE_SYMBOLX2_0(op4, &bitD4);
> - endSignal = (U32)LIKELY(
> + endSignal = (U32)LIKELY((U32)
> (BIT_reloadDStreamFast(&bitD1) == BIT_DStream_unfinished)
> & (BIT_reloadDStreamFast(&bitD2) == BIT_DStream_unfinished)
> & (BIT_reloadDStreamFast(&bitD3) == BIT_DStream_unfinished)
> --
> 2.33.1.637.gf443b226ca
>

\
 
 \ /
  Last update: 2021-10-26 03:17    [W:0.101 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site