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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sun, 29 Sep 2019, Julien Grall wrote:
> 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.

That's horrible, I didn't know about that!


> 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.

Is there a __REALLY_REALLY_GUNC__ variable? I guess not, so I don't have
a better suggestion. This problem is quite annoying (not your fault of
course) I wonder how other projects deal with it. There must be a
"clean" way to distinguish gcc from others?

For now, I am OK with this patch as is because I wouldn't know what else
to suggest, and I agree that !defined(__clang__) && defined(__GNUC__) is
bad.

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