Re: [PATCH v2 04/10] x86/percpu: Clean up percpu_add_op()

[Nick Desaulniers]

On Sat, May 30, 2020 at 3:11 PM Brian Gerst  wrote:
>
> The core percpu macros already have a switch on the data size, so the switch
> in the x86 code is redundant and produces more dead code.
>
> Also use appropriate types for the width of the instructions.  This avoids
> errors when compiling with Clang.
>
> Signed-off-by: Brian Gerst 

Reviewed-by: Nick Desaulniers 

> ---
>  arch/x86/include/asm/percpu.h | 99 ---
>  1 file changed, 22 insertions(+), 77 deletions(-)
>
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index a40d2e055f58..2a24f3c795eb 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -130,64 +130,32 @@ do {
>   \
> : [val] __pcpu_reg_imm_##size(pto_val__));  \
>  } while (0)
>
> +#define percpu_unary_op(size, qual, op, _var)  \
> +({ \
> +   asm qual (__pcpu_op1_##size(op, __percpu_arg([var]))\
> +   : [var] "+m" (_var));   \
> +})
> +
>  /*
>   * Generate a percpu add to memory instruction and optimize code
>   * if one is added or subtracted.
>   */
> -#define percpu_add_op(qual, var, val)  \
> +#define percpu_add_op(size, qual, var, val)\
>  do {   \
> -   typedef typeof(var) pao_T__;\
> const int pao_ID__ = (__builtin_constant_p(val) &&  \
>   ((val) == 1 || (val) == -1)) ?\
> (int)(val) : 0; \
> if (0) {\
> -   pao_T__ pao_tmp__;  \
> +   typeof(var) pao_tmp__;  \
> pao_tmp__ = (val);  \
> (void)pao_tmp__;\
> }   \
> -   switch (sizeof(var)) {  \
> -   case 1: \
> -   if (pao_ID__ == 1)  \
> -   asm qual ("incb "__percpu_arg(0) : "+m" (var)); \
> -   else if (pao_ID__ == -1)\
> -   asm qual ("decb "__percpu_arg(0) : "+m" (var)); \
> -   else\
> -   asm qual ("addb %1, "__percpu_arg(0)\
> -   : "+m" (var)\
> -   : "qi" ((pao_T__)(val)));   \
> -   break;  \
> -   case 2: \
> -   if (pao_ID__ == 1)  \
> -   asm qual ("incw "__percpu_arg(0) : "+m" (var)); \
> -   else if (pao_ID__ == -1)\
> -   asm qual ("decw "__percpu_arg(0) : "+m" (var)); \
> -   else\
> -   asm qual ("addw %1, "__percpu_arg(0)\
> -   : "+m" (var)\
> -   : "ri" ((pao_T__)(val)));   \
> -   break;  \
> -   case 4: \
> -   if (pao_ID__ == 1)  \
> -   asm qual ("incl "__percpu_arg(0) : "+m" (var)); \
> -   else if (pao_ID__ == -1)\
> -   asm qual ("decl "__percpu_arg(0) : "+m" (var)); \
> -   else\
> -   asm qual ("addl %1, "__percpu_arg(0)\
> -   : "+m" (var)\
> -   : "ri" ((pao_T__)(val)));   \
> -   break;  \
> -   case 8: \
> -   if (pao_ID__ == 1)  \
> -   asm qual ("incq "__percpu_arg(0) : "+m" (var)); \
> -   else if (pao_ID__ == -1)\
> - 

[PATCH v2 04/10] x86/percpu: Clean up percpu_add_op()

[Brian Gerst]

The core percpu macros already have a switch on the data size, so the switch
in the x86 code is redundant and produces more dead code.

Also use appropriate types for the width of the instructions.  This avoids
errors when compiling with Clang.

Signed-off-by: Brian Gerst 
---
 arch/x86/include/asm/percpu.h | 99 ---
 1 file changed, 22 insertions(+), 77 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a40d2e055f58..2a24f3c795eb 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -130,64 +130,32 @@ do {  
\
: [val] __pcpu_reg_imm_##size(pto_val__));  \
 } while (0)
 
+#define percpu_unary_op(size, qual, op, _var)  \
+({ \
+   asm qual (__pcpu_op1_##size(op, __percpu_arg([var]))\
+   : [var] "+m" (_var));   \
+})
+
 /*
  * Generate a percpu add to memory instruction and optimize code
  * if one is added or subtracted.
  */
-#define percpu_add_op(qual, var, val)  \
+#define percpu_add_op(size, qual, var, val)\
 do {   \
-   typedef typeof(var) pao_T__;\
const int pao_ID__ = (__builtin_constant_p(val) &&  \
  ((val) == 1 || (val) == -1)) ?\
(int)(val) : 0; \
if (0) {\
-   pao_T__ pao_tmp__;  \
+   typeof(var) pao_tmp__;  \
pao_tmp__ = (val);  \
(void)pao_tmp__;\
}   \
-   switch (sizeof(var)) {  \
-   case 1: \
-   if (pao_ID__ == 1)  \
-   asm qual ("incb "__percpu_arg(0) : "+m" (var)); \
-   else if (pao_ID__ == -1)\
-   asm qual ("decb "__percpu_arg(0) : "+m" (var)); \
-   else\
-   asm qual ("addb %1, "__percpu_arg(0)\
-   : "+m" (var)\
-   : "qi" ((pao_T__)(val)));   \
-   break;  \
-   case 2: \
-   if (pao_ID__ == 1)  \
-   asm qual ("incw "__percpu_arg(0) : "+m" (var)); \
-   else if (pao_ID__ == -1)\
-   asm qual ("decw "__percpu_arg(0) : "+m" (var)); \
-   else\
-   asm qual ("addw %1, "__percpu_arg(0)\
-   : "+m" (var)\
-   : "ri" ((pao_T__)(val)));   \
-   break;  \
-   case 4: \
-   if (pao_ID__ == 1)  \
-   asm qual ("incl "__percpu_arg(0) : "+m" (var)); \
-   else if (pao_ID__ == -1)\
-   asm qual ("decl "__percpu_arg(0) : "+m" (var)); \
-   else\
-   asm qual ("addl %1, "__percpu_arg(0)\
-   : "+m" (var)\
-   : "ri" ((pao_T__)(val)));   \
-   break;  \
-   case 8: \
-   if (pao_ID__ == 1)  \
-   asm qual ("incq "__percpu_arg(0) : "+m" (var)); \
-   else if (pao_ID__ == -1)\
-   asm qual ("decq "__percpu_arg(0) : "+m" (var)); \
-   else\
-   asm qual ("addq %1, "__percpu_arg(0)\
-   : "+m"