Re: [Xen-devel] [PATCH 02/12] xen/arm: fix get_cpu_info() when built with clang

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 27 Mar 2019, Julien Grall wrote:
> Clang understands the GCCism in use here, but still complains that sp is
> unitialised. In such cases, resort to the older versions of this code,
> which directly read sp into the temporary variable.
> 
> Note that we still keep the GCCism in the default case, as it causes GCC
> to create rather better assembly.
> 
> This is based on the x86 counterpart.

I understand this is based on an existing approach but what about other
compilers? I have a suggestion below.


> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/include/asm-arm/current.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> index c4af66fbb9..6b7c1df64d 100644
> --- a/xen/include/asm-arm/current.h
> +++ b/xen/include/asm-arm/current.h
> @@ -28,8 +28,16 @@ struct cpu_info {
>  
>  static inline struct cpu_info *get_cpu_info(void)
>  {
> +#ifdef __clang__
> +    unsigned long sp;
> +
> +    asm ("mov %0, sp" : "=r" (sp));
> +#else
>      register unsigned long sp asm ("sp");
> -    return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE - 
> sizeof(struct cpu_info));
> +#endif
> +
> +    return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) +
> +                               STACK_SIZE - sizeof(struct cpu_info));
>  }

I think it makes sense to switch the #ifdef around:

#ifdef __GNUC__
    /* gcc specific optimization */
#else
   /* regular implementation */
#endif

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