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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 30 Sep 2019, Stefano Stabellini wrote:
> 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.

and you can add:

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

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