All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: David Hildenbrand <david@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	KVM list <kvm@vger.kernel.org>,
	llvmlinux@lists.linuxfoundation.org,
	Alexander Potapenko <glider@google.com>,
	andreyknvl <andreyknvl@google.com>,
	Michael Davidson <md@google.com>,
	Greg Hackmann <ghackmann@google.com>,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang
Date: Tue, 12 Sep 2017 17:18:52 +0200	[thread overview]
Message-ID: <20170912151851.GA24313@flask> (raw)
In-Reply-To: <CACT4Y+boDPGU_02AhzUCCHfHdPaviMd5sTJTuV5RuYNor4wRpg@mail.gmail.com>

2017-09-12 16:42+0200, Dmitry Vyukov:
> Hi Radim,
> 
> I've just noticed that your commit "KVM: x86: generalize
> guest_cpuid_has_ helpers" breaks clang build on this assert:
> 
> static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
> {
>     unsigned x86_leaf = x86_feature / 32;
> 
>     BUILD_BUG_ON(!__builtin_constant_p(x86_leaf));
> 
> 
> In clang __builtin_constant_p is never true for function arguments,
> it's true only for compile-time constants (what you can use as stack
> array size, or C++ template argument). What would work is an
> additional macro along the lines of:

GCC optimizes it thanks to __always_inline, so the x86_feature is
constant in each instance of this function ... the goal is to have
compile-time input checking.

> #define x86_feature_cpuid(x) (BUILD_BUG_ON(!__builtin_constant_p(x),
> __x86_feature_cpuid(x))
> 
> But again assuming that caller pass the constant directly.

The __builtin_constant_p() check is just a canary, the important ones
are

  BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
  BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);

and these would be very awkward if moved out of the function.

> Could you please fix it?

Sure, I can just make them BUG_ON (or WARN_ON with error handling), but
I tried with clang version 4.0.1 and got no errors -- are you using an
older version?  (or a command other than `make HOSTCC=clang CC=clang`)

Thanks.

  reply	other threads:[~2017-09-12 15:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-12 14:42 "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Dmitry Vyukov
2017-09-12 15:18 ` Radim Krčmář [this message]
2017-09-12 15:51   ` Dmitry Vyukov
2017-09-12 15:54     ` Dmitry Vyukov
2017-09-12 16:03       ` Paolo Bonzini
2017-09-12 16:16         ` Dmitry Vyukov
2017-09-12 16:33           ` Paolo Bonzini
2017-09-12 17:33             ` Dmitry Vyukov
2017-09-13 10:58       ` Radim Krčmář

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170912151851.GA24313@flask \
    --to=rkrcmar@redhat.com \
    --cc=andreyknvl@google.com \
    --cc=david@redhat.com \
    --cc=dvyukov@google.com \
    --cc=ghackmann@google.com \
    --cc=glider@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvmlinux@lists.linuxfoundation.org \
    --cc=md@google.com \
    --cc=ndesaulniers@google.com \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.