Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH] lib: zstd: Add cast to silence clang's -Wbitwise-instead-of-logical | Date | Tue, 26 Oct 2021 10:34:31 +0000 |
| |
From: Nick Terrell > Sent: 26 October 2021 02:18 > > > 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. ... > > The first U32 cast is to silence an instance of -Wshorten-64-to-32 > > because __builtin_expect() returns long so it cannot be moved.
Isn't enabling that warning completely stupid? The casts required to silence it could easily cause more problems - by hiding more important bugs. And seriously affect code readability.
...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)
Isn't that the same as: ((BIT_reload() & BIT_reload() & BIT_reload()) == BIT_DStream_unfinished) which will generate much better code. Especially on cpu without 'seteq' instructions.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
| |