diff mbox series

[v5,1/7] bits: split the definition of the asm and non-asm GENMASK()

Message ID 20250306-fixed-type-genmasks-v5-1-b443e9dcba63@wanadoo.fr (mailing list archive)
State New
Headers show
Series bits: Fixed-type GENMASK()/BIT() | expand

Commit Message

Vincent Mailhol via B4 Relay March 6, 2025, 11:29 a.m. UTC 
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

In an upcoming change, GENMASK() and its friends will indirectly
depend on sizeof() which is not available in asm.

Instead of adding further complexity to __GENMASK() to make it work
for both asm and non asm, just split the definition of the two
variants.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Changelog:

  v4 -> v5:

    - Use tab indentations instead of single space to separate the
      macro name from its body.

  v3 -> v4:

    - New patch in the series
---
 include/linux/bits.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Vincent Mailhol March 6, 2025, 3:07 p.m. UTC | #1 
On 06/03/2025 at 22:05, Andy Shevchenko wrote:
> On Thu, Mar 06, 2025 at 08:29:52PM +0900, Vincent Mailhol via B4 Relay wrote:
>> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>>
>> In an upcoming change, GENMASK() and its friends will indirectly
>> depend on sizeof() which is not available in asm.
>>
>> Instead of adding further complexity to __GENMASK() to make it work
>> for both asm and non asm, just split the definition of the two
>> variants.
> 
> ...
> 
>> -/*
>> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
>> - * disable the input check if that is the case.
>> - */
> 
> I believe this comment is still valid...
> 
>> +#else /* defined(__ASSEMBLY__) */
> 
> 
> ...here.
> 
> Otherwise justify its removal in the commit message.

OK. I will restore the comment in v6, but will move it to the #else
branch, like this:

  #else /* defined(__ASSEMBLY__) */

  /*
   * BUILD_BUG_ON_ZERO is not available in h files included from asm
   * files, so no input checks in assembly.
   */
  #define GENMASK(h, l)		__GENMASK(h, l)
  #define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)

  #endif /* !defined(__ASSEMBLY__) */

>> +#define GENMASK(h, l)		__GENMASK(h, l)
>> +#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)
>> +
>> +#endif /* !defined(__ASSEMBLY__) */
> 

Yours sincerely,
Vincent Mailhol
David Laight March 6, 2025, 7:23 p.m. UTC | #2 
On Thu, 06 Mar 2025 20:29:52 +0900
Vincent Mailhol via B4 Relay <devnull+mailhol.vincent.wanadoo.fr@kernel.org> wrote:

> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> 
> In an upcoming change, GENMASK() and its friends will indirectly
> depend on sizeof() which is not available in asm.
> 
> Instead of adding further complexity to __GENMASK() to make it work
> for both asm and non asm, just split the definition of the two
> variants.
...
> +#else /* defined(__ASSEMBLY__) */
> +
> +#define GENMASK(h, l)		__GENMASK(h, l)
> +#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)

What do those actually expand to now?
As I've said a few times both UL(0) and ULL(0) are just (0) for __ASSEMBLY__
so the expansions of __GENMASK() and __GENMASK_ULL() contained the
same numeric constants.
This means they should be generating the same values.
I don't know the correct 'sizeof (int_type)' for the shift right of ~0.
My suspicion is that a 32bit assembler used 32bit signed integers and a
64bit one 64bit signed integers (but a 32bit asm on a 64bit host might
be 64bit).
So the asm versions need to avoid the right shift and only do left shifts.

Which probably means they need to be enirely separate from the C versions.
And then the C ones can have all the ULL() removed.

	David

> +
> +#endif /* !defined(__ASSEMBLY__) */
>  
>  #endif	/* __LINUX_BITS_H */
>
diff mbox series

Patch

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 14fd0ca9a6cd17339dd2f69e449558312a8a001b..4819cbe7bd48fbae796fc6005c9f92b1c93a034c 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -19,23 +19,17 @@ 
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
 #if !defined(__ASSEMBLY__)
+
 #include <linux/build_bug.h>
 #include <linux/compiler.h>
+
 #define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
-#else
-/*
- * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
- * disable the input check if that is the case.
- */
-#define GENMASK_INPUT_CHECK(h, l) 0
-#endif
 
 #define GENMASK(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 #define GENMASK_ULL(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
 
-#if !defined(__ASSEMBLY__)
 /*
  * Missing asm support
  *
@@ -48,6 +42,12 @@ 
  */
 #define GENMASK_U128(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
-#endif
+
+#else /* defined(__ASSEMBLY__) */
+
+#define GENMASK(h, l)		__GENMASK(h, l)
+#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)
+
+#endif /* !defined(__ASSEMBLY__) */
 
 #endif	/* __LINUX_BITS_H */