[PATCH] asm-generic: make sparse happy with odd-sized put_unaligned_*()

[PATCH] asm-generic: make sparse happy with odd-sized put_unaligned_*()

[Dmitry Torokhov]

__put_unaligned_be24() and friends use implicit casts to convert
larger-sized data to bytes, which trips sparse truncation warnings when
the argument is a constant:

  CC [M]  drivers/input/touchscreen/hynitron_cstxxx.o
  CHECK   drivers/input/touchscreen/hynitron_cstxxx.c
drivers/input/touchscreen/hynitron_cstxxx.c: note: in included file (through arch/x86/include/generated/asm/unaligned.h):
./include/asm-generic/unaligned.h:119:16: warning: cast truncates bits from constant value (aa01a0 becomes a0)
./include/asm-generic/unaligned.h:120:20: warning: cast truncates bits from constant value (aa01 becomes 1)
./include/asm-generic/unaligned.h:119:16: warning: cast truncates bits from constant value (ab00d0 becomes d0)
./include/asm-generic/unaligned.h:120:20: warning: cast truncates bits from constant value (ab00 becomes 0)

To avoid this let's mask off upper bits explicitly, the resulting code
should be exactly the same, but it will keep sparse happy.

Reported-by: kernel test robot <lkp@intel.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Closes: https://lore.kernel.org/oe-kbuild-all/202401070147.gqwVulOn-lkp@intel.com/
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 include/asm-generic/unaligned.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 699650f81970..a84c64e5f11e 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -104,9 +104,9 @@ static inline u32 get_unaligned_le24(const void *p)
 
 static inline void __put_unaligned_be24(const u32 val, u8 *p)
 {
-	*p++ = val >> 16;
-	*p++ = val >> 8;
-	*p++ = val;
+	*p++ = (val >> 16) & 0xff;
+	*p++ = (val >> 8) & 0xff;
+	*p++ = val & 0xff;
 }
 
 static inline void put_unaligned_be24(const u32 val, void *p)
@@ -116,9 +116,9 @@ static inline void put_unaligned_be24(const u32 val, void *p)
 
 static inline void __put_unaligned_le24(const u32 val, u8 *p)
 {
-	*p++ = val;
-	*p++ = val >> 8;
-	*p++ = val >> 16;
+	*p++ = val & 0xff;
+	*p++ = (val >> 8) & 0xff;
+	*p++ = (val >> 16) & 0xff;
 }
 
 static inline void put_unaligned_le24(const u32 val, void *p)
@@ -128,12 +128,12 @@ static inline void put_unaligned_le24(const u32 val, void *p)
 
 static inline void __put_unaligned_be48(const u64 val, u8 *p)
 {
-	*p++ = val >> 40;
-	*p++ = val >> 32;
-	*p++ = val >> 24;
-	*p++ = val >> 16;
-	*p++ = val >> 8;
-	*p++ = val;
+	*p++ = (val >> 40) & 0xff;
+	*p++ = (val >> 32) & 0xff;
+	*p++ = (val >> 24) & 0xff;
+	*p++ = (val >> 16) & 0xff;
+	*p++ = (val >> 8) & 0xff;
+	*p++ = val & 0xff;
 }
 
 static inline void put_unaligned_be48(const u64 val, void *p)
-- 
2.43.0.195.gebba966016-goog


-- 
Dmitry

RE: [PATCH] asm-generic: make sparse happy with odd-sized put_unaligned_*()

[David Laight]

From: Dmitry Torokhov
> Sent: 08 January 2024 06:17
> 
> __put_unaligned_be24() and friends use implicit casts to convert
> larger-sized data to bytes, which trips sparse truncation warnings when
> the argument is a constant:
> 
>   CC [M]  drivers/input/touchscreen/hynitron_cstxxx.o
>   CHECK   drivers/input/touchscreen/hynitron_cstxxx.c
> drivers/input/touchscreen/hynitron_cstxxx.c: note: in included file (through
> arch/x86/include/generated/asm/unaligned.h):
> ./include/asm-generic/unaligned.h:119:16: warning: cast truncates bits from constant value (aa01a0
> becomes a0)
> ./include/asm-generic/unaligned.h:120:20: warning: cast truncates bits from constant value (aa01
> becomes 1)
> ./include/asm-generic/unaligned.h:119:16: warning: cast truncates bits from constant value (ab00d0
> becomes d0)
> ./include/asm-generic/unaligned.h:120:20: warning: cast truncates bits from constant value (ab00
> becomes 0)
> 
> To avoid this let's mask off upper bits explicitly, the resulting code
> should be exactly the same, but it will keep sparse happy.

Maybe someone should fix sparse?
I have seen a compiler generate two explicit masks with 0xff
followed by a byte write for:
	*p = (char)(x & 0xff);
but I expect modern gcc is ok.

> Reported-by: kernel test robot <lkp@intel.com>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Closes: https://lore.kernel.org/oe-kbuild-all/202401070147.gqwVulOn-lkp@intel.com/
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  include/asm-generic/unaligned.h | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
> index 699650f81970..a84c64e5f11e 100644
> --- a/include/asm-generic/unaligned.h
> +++ b/include/asm-generic/unaligned.h
> @@ -104,9 +104,9 @@ static inline u32 get_unaligned_le24(const void *p)
> 
>  static inline void __put_unaligned_be24(const u32 val, u8 *p)
>  {
> -	*p++ = val >> 16;
> -	*p++ = val >> 8;
> -	*p++ = val;
> +	*p++ = (val >> 16) & 0xff;
> +	*p++ = (val >> 8) & 0xff;
> +	*p++ = val & 0xff;
>  }

What happens if you implement the as (eg):
	*p = val >> 16;
	put_unaligned_be16(p + 1, val);
I think that should generate better code.
And it may stop sparse bleating.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] asm-generic: make sparse happy with odd-sized put_unaligned_*()

['Dmitry Torokhov']

On Mon, Jan 08, 2024 at 11:03:22AM +0000, David Laight wrote:
> From: Dmitry Torokhov
> > Sent: 08 January 2024 06:17
> > 
> > __put_unaligned_be24() and friends use implicit casts to convert
> > larger-sized data to bytes, which trips sparse truncation warnings when
> > the argument is a constant:
> > 
> >   CC [M]  drivers/input/touchscreen/hynitron_cstxxx.o
> >   CHECK   drivers/input/touchscreen/hynitron_cstxxx.c
> > drivers/input/touchscreen/hynitron_cstxxx.c: note: in included file (through
> > arch/x86/include/generated/asm/unaligned.h):
> > ./include/asm-generic/unaligned.h:119:16: warning: cast truncates bits from constant value (aa01a0
> > becomes a0)
> > ./include/asm-generic/unaligned.h:120:20: warning: cast truncates bits from constant value (aa01
> > becomes 1)
> > ./include/asm-generic/unaligned.h:119:16: warning: cast truncates bits from constant value (ab00d0
> > becomes d0)
> > ./include/asm-generic/unaligned.h:120:20: warning: cast truncates bits from constant value (ab00
> > becomes 0)
> > 
> > To avoid this let's mask off upper bits explicitly, the resulting code
> > should be exactly the same, but it will keep sparse happy.
> 
> Maybe someone should fix sparse?

I proposed doing this in
https://lore.kernel.org/oe-kbuild-all/ZZnzd3s2L-ZwGOlz@google.com/ but
the idea was not welcome.

> I have seen a compiler generate two explicit masks with 0xff
> followed by a byte write for:
> 	*p = (char)(x & 0xff);
> but I expect modern gcc is ok.
> 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202401070147.gqwVulOn-lkp@intel.com/
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  include/asm-generic/unaligned.h | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
> > index 699650f81970..a84c64e5f11e 100644
> > --- a/include/asm-generic/unaligned.h
> > +++ b/include/asm-generic/unaligned.h
> > @@ -104,9 +104,9 @@ static inline u32 get_unaligned_le24(const void *p)
> > 
> >  static inline void __put_unaligned_be24(const u32 val, u8 *p)
> >  {
> > -	*p++ = val >> 16;
> > -	*p++ = val >> 8;
> > -	*p++ = val;
> > +	*p++ = (val >> 16) & 0xff;
> > +	*p++ = (val >> 8) & 0xff;
> > +	*p++ = val & 0xff;
> >  }
> 
> What happens if you implement the as (eg):
> 	*p = val >> 16;
> 	put_unaligned_be16(p + 1, val);
> I think that should generate better code.
> And it may stop sparse bleating.

This is rarely in a hot path (typically you do this with a "slow"
device), and while being faster it looks more complex. But if that's
what people prefer...

Thanks.

-- 
Dmitry