Messages in this thread | | | From | Nick Terrell <> | Subject | Re: [PATCH] lib: zstd: Add cast to silence clang's -Wbitwise-instead-of-logical | Date | Tue, 26 Oct 2021 01:17:30 +0000 |
| |
> 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 >
| |