[PATCH 1/2] uapi: Define GENMASK_U128

Anshuman Khandual posted 2 patches 1 year, 4 months ago
There is a newer version of this series
[PATCH 1/2] uapi: Define GENMASK_U128
Posted by Anshuman Khandual 1 year, 4 months ago
This adds GENMASK_U128() and __GENMASK_U128() macros using __BITS_PER_U128
and __int128 data types. These macros will be used in providing support for
generating 128 bit masks.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Arnd Bergmann <arnd@arndb.de>>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 include/linux/bits.h                   | 2 ++
 include/uapi/asm-generic/bitsperlong.h | 4 ++++
 include/uapi/linux/bits.h              | 4 ++++
 include/uapi/linux/const.h             | 3 +++
 4 files changed, 13 insertions(+)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 0eb24d21aac2..0a174cce09d2 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -35,5 +35,7 @@
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 #define GENMASK_ULL(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
+#define GENMASK_U128(h, l) \
+	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
 
 #endif	/* __LINUX_BITS_H */
diff --git a/include/uapi/asm-generic/bitsperlong.h b/include/uapi/asm-generic/bitsperlong.h
index fadb3f857f28..a2c425455b2f 100644
--- a/include/uapi/asm-generic/bitsperlong.h
+++ b/include/uapi/asm-generic/bitsperlong.h
@@ -28,4 +28,8 @@
 #define __BITS_PER_LONG_LONG 64
 #endif
 
+#ifndef __BITS_PER_U128
+#define __BITS_PER_U128 128
+#endif
+
 #endif /* _UAPI__ASM_GENERIC_BITS_PER_LONG */
diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
index 3c2a101986a3..780cf99b8e6e 100644
--- a/include/uapi/linux/bits.h
+++ b/include/uapi/linux/bits.h
@@ -12,4 +12,8 @@
         (((~_ULL(0)) - (_ULL(1) << (l)) + 1) & \
          (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
 
+#define __GENMASK_U128(h, l) \
+	(((~_U128(0)) - (_U128(1) << (l)) + 1) & \
+	 (~_U128(0) >> (__BITS_PER_U128 - 1 - (h))))
+
 #endif /* _UAPI_LINUX_BITS_H */
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index a429381e7ca5..e7d9189e6630 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -16,14 +16,17 @@
 #ifdef __ASSEMBLY__
 #define _AC(X,Y)	X
 #define _AT(T,X)	X
+#define _AC128(X)	X
 #else
 #define __AC(X,Y)	(X##Y)
 #define _AC(X,Y)	__AC(X,Y)
 #define _AT(T,X)	((T)(X))
+#define _AC128(X)	((unsigned __int128)(X))
 #endif
 
 #define _UL(x)		(_AC(x, UL))
 #define _ULL(x)		(_AC(x, ULL))
+#define _U128(x)	(_AC128(x))
 
 #define _BITUL(x)	(_UL(1) << (x))
 #define _BITULL(x)	(_ULL(1) << (x))
-- 
2.30.2
Re: [PATCH 1/2] uapi: Define GENMASK_U128
Posted by Arnd Bergmann 1 year, 4 months ago
On Wed, Jul 24, 2024, at 12:31, Anshuman Khandual wrote:
> --- a/include/uapi/asm-generic/bitsperlong.h
> +++ b/include/uapi/asm-generic/bitsperlong.h
> @@ -28,4 +28,8 @@
>  #define __BITS_PER_LONG_LONG 64
>  #endif
> 
> +#ifndef __BITS_PER_U128
> +#define __BITS_PER_U128 128
> +#endif

I would hope we don't need this definition. Not that it
hurts at all, but __BITS_PER_LONG_LONG was already kind
of pointless since we don't run on anything else and
__BITS_PER_U128 clearly can't have any other sensible
definition than a plain 128.

>  #define __AC(X,Y)	(X##Y)
>  #define _AC(X,Y)	__AC(X,Y)
>  #define _AT(T,X)	((T)(X))
> +#define _AC128(X)	((unsigned __int128)(X))

I just tried using this syntax and it doesn't seem to do
what you expected. gcc silently truncates the constant
to a 64-bit value here, while clang fails the build.
See also https://godbolt.org/z/rzEqra7nY
https://stackoverflow.com/questions/63328802/unsigned-int128-literal-gcc

The __GENMASK_U128() macro however seems to work correctly
since you start out with a smaller number and then shift
it after the type conversion.

     Arnd
Re: [PATCH 1/2] uapi: Define GENMASK_U128
Posted by Anshuman Khandual 1 year, 4 months ago

On 7/24/24 16:33, Arnd Bergmann wrote:
> On Wed, Jul 24, 2024, at 12:31, Anshuman Khandual wrote:
>> --- a/include/uapi/asm-generic/bitsperlong.h
>> +++ b/include/uapi/asm-generic/bitsperlong.h
>> @@ -28,4 +28,8 @@
>>  #define __BITS_PER_LONG_LONG 64
>>  #endif
>>
>> +#ifndef __BITS_PER_U128
>> +#define __BITS_PER_U128 128
>> +#endif
> 
> I would hope we don't need this definition. Not that it
> hurts at all, but __BITS_PER_LONG_LONG was already kind
> of pointless since we don't run on anything else and
> __BITS_PER_U128 clearly can't have any other sensible
> definition than a plain 128.

Agreed, although this just followed __BITS_PER_LONG_LONG.
But sure __BITS_PER_U128 can be plain 128.

So would you like to have #ifndef __BITS_PER_LONG_LONG dropped here 
as well ? But should that be folded or in a separate patch ?

> 
>>  #define __AC(X,Y)	(X##Y)
>>  #define _AC(X,Y)	__AC(X,Y)
>>  #define _AT(T,X)	((T)(X))
>> +#define _AC128(X)	((unsigned __int128)(X))
> 
> I just tried using this syntax and it doesn't seem to do
> what you expected. gcc silently truncates the constant

But numbers passed into _AC128() are smaller in the range [128..0].
Hence the truncation might not be problematic in this context ? OR
could it be ?

> to a 64-bit value here, while clang fails the build.

Should this be disabled for CC_IS_CLANG ?

> See also https://godbolt.org/z/rzEqra7nY
> https://stackoverflow.com/questions/63328802/unsigned-int128-literal-gcc

So unless the value in there is beyond 64 bits, it should be good ?
OR am I missing something.

> 
> The __GENMASK_U128() macro however seems to work correctly
> since you start out with a smaller number and then shift
> it after the type conversion.

_U128() never receives anything beyond [127..0] range. So then this
should be good ?
Re: [PATCH 1/2] uapi: Define GENMASK_U128
Posted by Arnd Bergmann 1 year, 4 months ago
On Wed, Jul 24, 2024, at 13:59, Anshuman Khandual wrote:
> On 7/24/24 16:33, Arnd Bergmann wrote:
>> I would hope we don't need this definition. Not that it
>> hurts at all, but __BITS_PER_LONG_LONG was already kind
>> of pointless since we don't run on anything else and
>> __BITS_PER_U128 clearly can't have any other sensible
>> definition than a plain 128.
>
> Agreed, although this just followed __BITS_PER_LONG_LONG.
> But sure __BITS_PER_U128 can be plain 128.
>
> So would you like to have #ifndef __BITS_PER_LONG_LONG dropped here 
> as well ? But should that be folded or in a separate patch ?

A separate patch is probably better, but you can also
just leave it.

>>>  #define __AC(X,Y)	(X##Y)
>>>  #define _AC(X,Y)	__AC(X,Y)
>>>  #define _AT(T,X)	((T)(X))
>>> +#define _AC128(X)	((unsigned __int128)(X))
>> 
>> I just tried using this syntax and it doesn't seem to do
>> what you expected. gcc silently truncates the constant
>
> But numbers passed into _AC128() are smaller in the range [128..0].
> Hence the truncation might not be problematic in this context ? OR
> could it be ?
>
>> to a 64-bit value here, while clang fails the build.
>
> Should this be disabled for CC_IS_CLANG ?
>
>> See also https://godbolt.org/z/rzEqra7nY
>> https://stackoverflow.com/questions/63328802/unsigned-int128-literal-gcc
>
> So unless the value in there is beyond 64 bits, it should be good ?
> OR am I missing something.
>
>> The __GENMASK_U128() macro however seems to work correctly
>> since you start out with a smaller number and then shift
>> it after the type conversion.
>
> _U128() never receives anything beyond [127..0] range. So then this
> should be good ?

Since you define _U128() right next to _ULL(), I would argue
that it should have the corresponding behavior for any value
that can fit into the type. Since that is currently not
possible with gcc, I would prefer to not define it at all.

However, I think you can just define a _BIT128() macro
that behaves the same way as _BITULL() and define
__GENMASK_U128() based on that. Maybe something like

#define _BIT128(x) ((unsigned __int128)1 << (x))
#define __GENMASK_U128(h, l) (_BIT128((h) + 1)) - (_BIT128(l))

     Arnd