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

From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Artem_Mygaiev@epam.com, xen-devel@lists.xenproject.org,
	Andrii_Anisov@epam.com, Oleksandr_Tyshchenko@epam.com
Subject: Re: [Xen-devel] [PATCH 02/12] xen/arm: fix get_cpu_info() when built with clang
Date: Sun, 29 Sep 2019 15:26:09 +0100	[thread overview]
Message-ID: <441ce7f6-7907-dc23-b0e6-7503ba888953@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1904181056410.1370@sstabellini-ThinkPad-X260>

Hi,

Sorry, I am picking up this series again.

On 4/18/19 7:03 PM, Stefano Stabellini wrote:
> On Wed, 17 Apr 2019, Julien Grall wrote:
>> Hi,
>>
>> On 4/17/19 9:45 PM, Stefano Stabellini wrote:
>>> 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.
>>
>> What if the compiler actually support named registers? Why would we make the
>> code less efficient?
> 
> It is not my intention to make the code less efficient for other
> compilers. However, reading the commit message and the patch I have the
> impression that the clang version is more likely to be applicable to
> other compilers, compared to the gcc version. More "standard". The
> reason is that the clang version only requires asm inline, while the gcc
> version requires both asm inline and named registers. For the sake of
> getting Xen to compile out of the box with any C compiler, I think it is
> best if we default to the less demanding version of the implementation
> for unknown compilers.
While building Xen out of box is nice goal to have, this is likely be 
very hard to reach out because Xen is using a lot of GCCism. It mostly 
work with Clang because they have adopted some of them.

I would be happy to revert the condition, but then AFAICT there are no 
pretty way to now that we are using GCC. While the define __GNUC__ is 
meant to tell you this is compiled with GCC, clang is also defining it.

So the condition would have to be

#if !defined(__clang__) && defined(__GNUC__)

But then if clang is already defining __GNUC__, what actually prevents 
any other to do it?

I have yet to see anyone wanted to build Xen with another compiler other 
than clang and GCC. So I will leave this patch as is. Feel free to 
suggest a different approach if you are not happy with this.

Cheers,

-- 
Julien Grall

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