LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
To: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	linux@rasmusvillemoes.dk, andriy.shevchenko@intel.com,
	dan.j.williams@intel.com, peterz@infradead.org
Subject: [PATCH v6 1/2] x86: fix bitops.h warning with a moved cast
Date: Tue, 10 Mar 2020 15:17:46 -0700	[thread overview]
Message-ID: <20200310221747.2848474-1-jesse.brandeburg@intel.com> (raw)

Fix many sparse warnings when building with C=1. These are useless
noise from the bitops.h file and getting rid of them helps devs
make more use of the tools and possibly find real bugs.

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), 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 warning was occurring because certain bitmasks that end with a bit
set next to a natural boundary like 7, 15, 23, 31, end up with a mask
like 0x7f, which then results in sign extension due to the integer
type promotion rules[1]. It was really only "clear_bit" that was
having problems, and it was only on some bit checks that resulted in a
mask like 0xffffff7f being generated after the inversion.

Verified with a test module (see next patch) and assembly inspection
that the patch doesn't introduce any change in generated code.

[1] https://stackoverflow.com/questions/46073295/implicit-type-promotion-rules

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
v6: reworded commit message, enhanced explanation
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: 8b614cb8f1dcac8ca77cf4dd85f46ef3055f8238
-- 
2.24.1


             reply	other threads:[~2020-03-10 22:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-10 22:17 Jesse Brandeburg [this message]
2020-03-10 22:17 ` [PATCH v6 2/2] lib: make a test module with set/clear bit Jesse Brandeburg
2020-04-15 14:46   ` Andy Shevchenko
2020-03-11  4:37 ` [PATCH v6 1/2] x86: fix bitops.h warning with a moved cast Luc Van Oostenryck
2020-03-17 19:25 ` Peter Zijlstra
2020-03-18 21:59 ` [tip: x86/asm] x86: Fix " tip-bot2 for Jesse Brandeburg
2020-05-04 19:37 ` [PATCH v6 1/2] x86: fix " Nick Desaulniers
2020-05-05  1:14   ` Jesse Brandeburg
2020-05-05 15:14     ` Andy Shevchenko
2020-05-05 17:29       ` Nick Desaulniers
2020-05-05 17:47         ` Nick Desaulniers

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=20200310221747.2848474-1-jesse.brandeburg@intel.com \
    --to=jesse.brandeburg@intel.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH v6 1/2] x86: fix bitops.h warning with a moved cast' \
    /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

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 NNTP newsgroup(s).