Messages in this thread Patch in this message | | | Date | Tue, 25 Feb 2020 12:30:50 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v5 1/2] x86: fix bitops.h warning with a moved cast |
| |
On Mon, Feb 24, 2020 at 02:50:19PM -0800, Jesse Brandeburg wrote: > Fix many sparse warnings when building with C=1. > > When the kernel is compiled with C=1, there are lots of messages like: > arch/x86/include/asm/bitops.h:77:37: warning: cast truncates bits from constant value (ffffff7f becomes 7f) > > CONST_MASK() is using a signed integer "1" to create the mask which > is later cast to (u8) when used, in order to yield an 8-bit value > for the assembly instructions to use. Simplify the expressions used to > clearly indicate they are working on 8-bit values only, which still > keeps sparse happy without an accidental promotion to a 32 bit integer. >
> The reason the warning was occurring is because certain bitmasks that > end with a mask next to a natural boundary like 7, 15, 23, 31, end up > with a mask like 0x7f, which then results in sign extension when doing > an invert (but I'm not a compiler expert). It was really only > "clear_bit" that was having problems, and it was only on bit checks next > to a byte boundary (top bit).
I guess this describes it incorrectly. The problem is integer promotion of negation operation for any bit basically.
For example, if we have bit 3 (nr = 3) and got it in the clear bit we will get (according to what I see in the warnings) 0xfffffff7. Which is simple ~(1 << 3) promoted to integer.
I think it is a C standard which dictates this, compiler just follows.
> Verified with a test module (see next patch) and assembly inspection > that the patch doesn't introduce any change in generated code. > > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > --- > v5: changed code to use simple AND and XOR, updated commit message > v4: reverse argument order as suggested by David Laight, added reviewed-by > v3: Clean up the header file changes as per peterz. > v2: use correct CC: list > --- > arch/x86/include/asm/bitops.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h > index 062cdecb2f24..53f246e9df5a 100644 > --- a/arch/x86/include/asm/bitops.h > +++ b/arch/x86/include/asm/bitops.h > @@ -54,7 +54,7 @@ arch_set_bit(long nr, volatile unsigned long *addr) > if (__builtin_constant_p(nr)) { > asm volatile(LOCK_PREFIX "orb %1,%0" > : CONST_MASK_ADDR(nr, addr) > - : "iq" ((u8)CONST_MASK(nr)) > + : "iq" (CONST_MASK(nr) & 0xff) > : "memory"); > } else { > asm volatile(LOCK_PREFIX __ASM_SIZE(bts) " %1,%0" > @@ -74,7 +74,7 @@ arch_clear_bit(long nr, volatile unsigned long *addr) > if (__builtin_constant_p(nr)) { > asm volatile(LOCK_PREFIX "andb %1,%0" > : CONST_MASK_ADDR(nr, addr) > - : "iq" ((u8)~CONST_MASK(nr))); > + : "iq" (CONST_MASK(nr) ^ 0xff)); > } else { > asm volatile(LOCK_PREFIX __ASM_SIZE(btr) " %1,%0" > : : RLONG_ADDR(addr), "Ir" (nr) : "memory"); > > base-commit: ca7e1fd1026c5af6a533b4b5447e1d2f153e28f2 > -- > 2.24.1 >
-- With Best Regards, Andy Shevchenko
| |