[Xen-devel] [PATCH] xen/arm: Workaround clang/armclang support for register allocation

Julien Grall posted 1 patch 4 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200217222034.11949-1-julien@xen.org
xen/include/asm-arm/asm_defns.h |  8 +++-
xen/include/asm-arm/smccc.h     | 74 ++++++++++++++++-----------------
2 files changed, 44 insertions(+), 38 deletions(-)
[Xen-devel] [PATCH] xen/arm: Workaround clang/armclang support for register allocation
Posted by Julien Grall 4 years, 2 months ago
Clang 8.0 (see [1]) and by extent some of the version of armclang does
not support register allocation using the syntax rN.

Thankfully, both GCC [2] and clang are able to support the xN syntax for
Arm64. Introduce a new macro ASM_REG() and use in common code for
register allocation.

[1] https://reviews.llvm.org/rL328829
[2] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html

Cc: Andrii Anisov <andrii_anisov@epam.com>
Signed-off-by: Julien Grall <julien@xen.org>
---
 xen/include/asm-arm/asm_defns.h |  8 +++-
 xen/include/asm-arm/smccc.h     | 74 ++++++++++++++++-----------------
 2 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/xen/include/asm-arm/asm_defns.h b/xen/include/asm-arm/asm_defns.h
index b4fbcdae1d..29a9dbb002 100644
--- a/xen/include/asm-arm/asm_defns.h
+++ b/xen/include/asm-arm/asm_defns.h
@@ -7,11 +7,17 @@
 #endif
 #include <asm/processor.h>
 
-/* For generic assembly code: use macros to define operand sizes. */
+/* Macros for generic assembly code */
 #if defined(CONFIG_ARM_32)
 # define __OP32
+# define ASM_REG(index) asm("r" # index)
 #elif defined(CONFIG_ARM_64)
 # define __OP32 "w"
+/*
+ * Clang < 8.0 doesn't support register alllocation using the syntax rN.
+ * See https://reviews.llvm.org/rL328829.
+ */
+# define ASM_REG(index) asm("x" # index)
 #else
 # error "unknown ARM variant"
 #endif
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 126399dd70..9d94beb3df 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -120,59 +120,59 @@ struct arm_smccc_res {
 #define __constraint_read_6 __constraint_read_5, "r" (r6)
 #define __constraint_read_7 __constraint_read_6, "r" (r7)
 
-#define __declare_arg_0(a0, res)                        \
-    struct arm_smccc_res    *___res = res;              \
-    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
-    register unsigned long  r1 asm("r1");               \
-    register unsigned long  r2 asm("r2");               \
-    register unsigned long  r3 asm("r3")
-
-#define __declare_arg_1(a0, a1, res)                    \
-    typeof(a1) __a1 = a1;                               \
-    struct arm_smccc_res    *___res = res;              \
-    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
-    register unsigned long  r1 asm("r1") = __a1;        \
-    register unsigned long  r2 asm("r2");               \
-    register unsigned long  r3 asm("r3")
-
-#define __declare_arg_2(a0, a1, a2, res)                \
-    typeof(a1) __a1 = a1;                               \
-    typeof(a2) __a2 = a2;                               \
-    struct arm_smccc_res    *___res = res;				\
-    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
-    register unsigned long  r1 asm("r1") = __a1;        \
-    register unsigned long  r2 asm("r2") = __a2;        \
-    register unsigned long  r3 asm("r3")
-
-#define __declare_arg_3(a0, a1, a2, a3, res)            \
-    typeof(a1) __a1 = a1;                               \
-    typeof(a2) __a2 = a2;                               \
-    typeof(a3) __a3 = a3;                               \
-    struct arm_smccc_res    *___res = res;              \
-    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
-    register unsigned long  r1 asm("r1") = __a1;        \
-    register unsigned long  r2 asm("r2") = __a2;        \
-    register unsigned long  r3 asm("r3") = __a3
+#define __declare_arg_0(a0, res)                            \
+    struct arm_smccc_res    *___res = res;                  \
+    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
+    register unsigned long  r1 ASM_REG(1);                  \
+    register unsigned long  r2 ASM_REG(2);                  \
+    register unsigned long  r3 ASM_REG(3)
+
+#define __declare_arg_1(a0, a1, res)                        \
+    typeof(a1) __a1 = a1;                                   \
+    struct arm_smccc_res    *___res = res;                  \
+    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
+    register unsigned long  r1 ASM_REG(1) = __a1;           \
+    register unsigned long  r2 ASM_REG(2);                  \
+    register unsigned long  r3 ASM_REG(3)
+
+#define __declare_arg_2(a0, a1, a2, res)                    \
+    typeof(a1) __a1 = a1;                                   \
+    typeof(a2) __a2 = a2;                                   \
+    struct arm_smccc_res    *___res = res;				    \
+    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
+    register unsigned long  r1 ASM_REG(1) = __a1;           \
+    register unsigned long  r2 ASM_REG(2) = __a2;           \
+    register unsigned long  r3 ASM_REG(3)
+
+#define __declare_arg_3(a0, a1, a2, a3, res)                \
+    typeof(a1) __a1 = a1;                                   \
+    typeof(a2) __a2 = a2;                                   \
+    typeof(a3) __a3 = a3;                                   \
+    struct arm_smccc_res    *___res = res;                  \
+    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
+    register unsigned long  r1 ASM_REG(1) = __a1;           \
+    register unsigned long  r2 ASM_REG(2) = __a2;           \
+    register unsigned long  r3 ASM_REG(3) = __a3
 
 #define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
     typeof(a4) __a4 = a4;                               \
     __declare_arg_3(a0, a1, a2, a3, res);               \
-    register unsigned long r4 asm("r4") = __a4
+    register unsigned long r4 ASM_REG(4) = __a4
 
 #define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
     typeof(a5) __a5 = a5;                               \
     __declare_arg_4(a0, a1, a2, a3, a4, res);           \
-    register typeof(a5) r5 asm("r5") = __a5
+    register typeof(a5) r5 ASM_REG(5) = __a5
 
 #define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
     typeof(a6) __a6 = a6;                                   \
     __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
-    register typeof(a6) r6 asm("r6") = __a6
+    register typeof(a6) r6 ASM_REG(6) = __a6
 
 #define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
     typeof(a7) __a7 = a7;                                       \
     __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
-    register typeof(a7) r7 asm("r7") = __a7
+    register typeof(a7) r7 ASM_REG(7) = __a7
 
 #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
 #define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Workaround clang/armclang support for register allocation
Posted by Julien Grall 4 years, 1 month ago
Hi,

Ping?

Cheers,

On 17/02/2020 22:20, Julien Grall wrote:
> Clang 8.0 (see [1]) and by extent some of the version of armclang does
> not support register allocation using the syntax rN.
> 
> Thankfully, both GCC [2] and clang are able to support the xN syntax for
> Arm64. Introduce a new macro ASM_REG() and use in common code for
> register allocation.
> 
> [1] https://reviews.llvm.org/rL328829
> [2] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html
> 
> Cc: Andrii Anisov <andrii_anisov@epam.com>
> Signed-off-by: Julien Grall <julien@xen.org>
> ---
>   xen/include/asm-arm/asm_defns.h |  8 +++-
>   xen/include/asm-arm/smccc.h     | 74 ++++++++++++++++-----------------
>   2 files changed, 44 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/include/asm-arm/asm_defns.h b/xen/include/asm-arm/asm_defns.h
> index b4fbcdae1d..29a9dbb002 100644
> --- a/xen/include/asm-arm/asm_defns.h
> +++ b/xen/include/asm-arm/asm_defns.h
> @@ -7,11 +7,17 @@
>   #endif
>   #include <asm/processor.h>
>   
> -/* For generic assembly code: use macros to define operand sizes. */
> +/* Macros for generic assembly code */
>   #if defined(CONFIG_ARM_32)
>   # define __OP32
> +# define ASM_REG(index) asm("r" # index)
>   #elif defined(CONFIG_ARM_64)
>   # define __OP32 "w"
> +/*
> + * Clang < 8.0 doesn't support register alllocation using the syntax rN.
> + * See https://reviews.llvm.org/rL328829.
> + */
> +# define ASM_REG(index) asm("x" # index)
>   #else
>   # error "unknown ARM variant"
>   #endif
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 126399dd70..9d94beb3df 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -120,59 +120,59 @@ struct arm_smccc_res {
>   #define __constraint_read_6 __constraint_read_5, "r" (r6)
>   #define __constraint_read_7 __constraint_read_6, "r" (r7)
>   
> -#define __declare_arg_0(a0, res)                        \
> -    struct arm_smccc_res    *___res = res;              \
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1");               \
> -    register unsigned long  r2 asm("r2");               \
> -    register unsigned long  r3 asm("r3")
> -
> -#define __declare_arg_1(a0, a1, res)                    \
> -    typeof(a1) __a1 = a1;                               \
> -    struct arm_smccc_res    *___res = res;              \
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1") = __a1;        \
> -    register unsigned long  r2 asm("r2");               \
> -    register unsigned long  r3 asm("r3")
> -
> -#define __declare_arg_2(a0, a1, a2, res)                \
> -    typeof(a1) __a1 = a1;                               \
> -    typeof(a2) __a2 = a2;                               \
> -    struct arm_smccc_res    *___res = res;				\
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1") = __a1;        \
> -    register unsigned long  r2 asm("r2") = __a2;        \
> -    register unsigned long  r3 asm("r3")
> -
> -#define __declare_arg_3(a0, a1, a2, a3, res)            \
> -    typeof(a1) __a1 = a1;                               \
> -    typeof(a2) __a2 = a2;                               \
> -    typeof(a3) __a3 = a3;                               \
> -    struct arm_smccc_res    *___res = res;              \
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1") = __a1;        \
> -    register unsigned long  r2 asm("r2") = __a2;        \
> -    register unsigned long  r3 asm("r3") = __a3
> +#define __declare_arg_0(a0, res)                            \
> +    struct arm_smccc_res    *___res = res;                  \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1);                  \
> +    register unsigned long  r2 ASM_REG(2);                  \
> +    register unsigned long  r3 ASM_REG(3)
> +
> +#define __declare_arg_1(a0, a1, res)                        \
> +    typeof(a1) __a1 = a1;                                   \
> +    struct arm_smccc_res    *___res = res;                  \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1) = __a1;           \
> +    register unsigned long  r2 ASM_REG(2);                  \
> +    register unsigned long  r3 ASM_REG(3)
> +
> +#define __declare_arg_2(a0, a1, a2, res)                    \
> +    typeof(a1) __a1 = a1;                                   \
> +    typeof(a2) __a2 = a2;                                   \
> +    struct arm_smccc_res    *___res = res;				    \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1) = __a1;           \
> +    register unsigned long  r2 ASM_REG(2) = __a2;           \
> +    register unsigned long  r3 ASM_REG(3)
> +
> +#define __declare_arg_3(a0, a1, a2, a3, res)                \
> +    typeof(a1) __a1 = a1;                                   \
> +    typeof(a2) __a2 = a2;                                   \
> +    typeof(a3) __a3 = a3;                                   \
> +    struct arm_smccc_res    *___res = res;                  \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1) = __a1;           \
> +    register unsigned long  r2 ASM_REG(2) = __a2;           \
> +    register unsigned long  r3 ASM_REG(3) = __a3
>   
>   #define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
>       typeof(a4) __a4 = a4;                               \
>       __declare_arg_3(a0, a1, a2, a3, res);               \
> -    register unsigned long r4 asm("r4") = __a4
> +    register unsigned long r4 ASM_REG(4) = __a4
>   
>   #define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
>       typeof(a5) __a5 = a5;                               \
>       __declare_arg_4(a0, a1, a2, a3, a4, res);           \
> -    register typeof(a5) r5 asm("r5") = __a5
> +    register typeof(a5) r5 ASM_REG(5) = __a5
>   
>   #define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
>       typeof(a6) __a6 = a6;                                   \
>       __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
> -    register typeof(a6) r6 asm("r6") = __a6
> +    register typeof(a6) r6 ASM_REG(6) = __a6
>   
>   #define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
>       typeof(a7) __a7 = a7;                                       \
>       __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> -    register typeof(a7) r7 asm("r7") = __a7
> +    register typeof(a7) r7 ASM_REG(7) = __a7
>   
>   #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
>   #define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: Workaround clang/armclang support for register allocation
Posted by Stefano Stabellini 4 years, 1 month ago
On Mon, 17 Feb 2020, Julien Grall wrote:
> Clang 8.0 (see [1]) and by extent some of the version of armclang does
> not support register allocation using the syntax rN.
> 
> Thankfully, both GCC [2] and clang are able to support the xN syntax for
> Arm64. Introduce a new macro ASM_REG() and use in common code for
> register allocation.
> 
> [1] https://reviews.llvm.org/rL328829
> [2] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html
> 
> Cc: Andrii Anisov <andrii_anisov@epam.com>
> Signed-off-by: Julien Grall <julien@xen.org>

Thanks for the patch, it looks reasonable and the approach looks OK to
me.

Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/include/asm-arm/asm_defns.h |  8 +++-
>  xen/include/asm-arm/smccc.h     | 74 ++++++++++++++++-----------------
>  2 files changed, 44 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/include/asm-arm/asm_defns.h b/xen/include/asm-arm/asm_defns.h
> index b4fbcdae1d..29a9dbb002 100644
> --- a/xen/include/asm-arm/asm_defns.h
> +++ b/xen/include/asm-arm/asm_defns.h
> @@ -7,11 +7,17 @@
>  #endif
>  #include <asm/processor.h>
>  
> -/* For generic assembly code: use macros to define operand sizes. */
> +/* Macros for generic assembly code */
>  #if defined(CONFIG_ARM_32)
>  # define __OP32
> +# define ASM_REG(index) asm("r" # index)
>  #elif defined(CONFIG_ARM_64)
>  # define __OP32 "w"
> +/*
> + * Clang < 8.0 doesn't support register alllocation using the syntax rN.
> + * See https://reviews.llvm.org/rL328829.
> + */
> +# define ASM_REG(index) asm("x" # index)
>  #else
>  # error "unknown ARM variant"
>  #endif
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 126399dd70..9d94beb3df 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -120,59 +120,59 @@ struct arm_smccc_res {
>  #define __constraint_read_6 __constraint_read_5, "r" (r6)
>  #define __constraint_read_7 __constraint_read_6, "r" (r7)
>  
> -#define __declare_arg_0(a0, res)                        \
> -    struct arm_smccc_res    *___res = res;              \
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1");               \
> -    register unsigned long  r2 asm("r2");               \
> -    register unsigned long  r3 asm("r3")
> -
> -#define __declare_arg_1(a0, a1, res)                    \
> -    typeof(a1) __a1 = a1;                               \
> -    struct arm_smccc_res    *___res = res;              \
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1") = __a1;        \
> -    register unsigned long  r2 asm("r2");               \
> -    register unsigned long  r3 asm("r3")
> -
> -#define __declare_arg_2(a0, a1, a2, res)                \
> -    typeof(a1) __a1 = a1;                               \
> -    typeof(a2) __a2 = a2;                               \
> -    struct arm_smccc_res    *___res = res;				\
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1") = __a1;        \
> -    register unsigned long  r2 asm("r2") = __a2;        \
> -    register unsigned long  r3 asm("r3")
> -
> -#define __declare_arg_3(a0, a1, a2, a3, res)            \
> -    typeof(a1) __a1 = a1;                               \
> -    typeof(a2) __a2 = a2;                               \
> -    typeof(a3) __a3 = a3;                               \
> -    struct arm_smccc_res    *___res = res;              \
> -    register unsigned long  r0 asm("r0") = (uint32_t)a0;\
> -    register unsigned long  r1 asm("r1") = __a1;        \
> -    register unsigned long  r2 asm("r2") = __a2;        \
> -    register unsigned long  r3 asm("r3") = __a3
> +#define __declare_arg_0(a0, res)                            \
> +    struct arm_smccc_res    *___res = res;                  \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1);                  \
> +    register unsigned long  r2 ASM_REG(2);                  \
> +    register unsigned long  r3 ASM_REG(3)
> +
> +#define __declare_arg_1(a0, a1, res)                        \
> +    typeof(a1) __a1 = a1;                                   \
> +    struct arm_smccc_res    *___res = res;                  \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1) = __a1;           \
> +    register unsigned long  r2 ASM_REG(2);                  \
> +    register unsigned long  r3 ASM_REG(3)
> +
> +#define __declare_arg_2(a0, a1, a2, res)                    \
> +    typeof(a1) __a1 = a1;                                   \
> +    typeof(a2) __a2 = a2;                                   \
> +    struct arm_smccc_res    *___res = res;				    \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1) = __a1;           \
> +    register unsigned long  r2 ASM_REG(2) = __a2;           \
> +    register unsigned long  r3 ASM_REG(3)
> +
> +#define __declare_arg_3(a0, a1, a2, a3, res)                \
> +    typeof(a1) __a1 = a1;                                   \
> +    typeof(a2) __a2 = a2;                                   \
> +    typeof(a3) __a3 = a3;                                   \
> +    struct arm_smccc_res    *___res = res;                  \
> +    register unsigned long  r0 ASM_REG(0) = (uint32_t)a0;   \
> +    register unsigned long  r1 ASM_REG(1) = __a1;           \
> +    register unsigned long  r2 ASM_REG(2) = __a2;           \
> +    register unsigned long  r3 ASM_REG(3) = __a3
>  
>  #define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
>      typeof(a4) __a4 = a4;                               \
>      __declare_arg_3(a0, a1, a2, a3, res);               \
> -    register unsigned long r4 asm("r4") = __a4
> +    register unsigned long r4 ASM_REG(4) = __a4
>  
>  #define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
>      typeof(a5) __a5 = a5;                               \
>      __declare_arg_4(a0, a1, a2, a3, a4, res);           \
> -    register typeof(a5) r5 asm("r5") = __a5
> +    register typeof(a5) r5 ASM_REG(5) = __a5
>  
>  #define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
>      typeof(a6) __a6 = a6;                                   \
>      __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
> -    register typeof(a6) r6 asm("r6") = __a6
> +    register typeof(a6) r6 ASM_REG(6) = __a6
>  
>  #define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
>      typeof(a7) __a7 = a7;                                       \
>      __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> -    register typeof(a7) r7 asm("r7") = __a7
> +    register typeof(a7) r7 ASM_REG(7) = __a7
>  
>  #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
>  #define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
> -- 
> 2.17.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel