Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 12, 2017 at 6:03 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> On 12/09/2017 17:54, Dmitry Vyukov wrote:
>>> I guess clang still eliminates dead branches. Clang optimizer does
>>> know that these are constant, it just does not allow build
>>> success/failure nor runtime behavior depend on optimization level and
>>> compiler version. I.e. with gcc you can get build failure with only
>>> some compiler flags and/or compiler versions. Clang gives stable
>>> result. But the optimizer does use constant propagation, etc during
>>> optimization.
>
> I can reproduce it:
>
> $ cat f.c
> int bad_code();
>
> static inline void __attribute__((always_inline)) f(int x)
> {
>         if (!__builtin_constant_p(x))
>                 bad_code();
> }
>
> int main()
> {
>         f(0);
>         f(1);
>         f(100);
> }
>
> $ clang --version
> clang version 4.0.0 (tags/RELEASE_400/final)
> $ clang f.c -O2 -c -o f.o
> $ nm f.o
>                  U bad_code
> 0000000000000000 T main
>
> $ gcc f.c -O2 -c -o f.o
> $ nm f.o
> 0000000000000000 T main
>
> ... but I don't know, it seems very weird.  The purpose of
> __builtin_constant_p is to be resolved only relatively late in the
> optimization pipeline, and it has been like this for at least 15 years
> in GCC.
>
> The docs say what to expect:
>
>   You may use this built-in function in either a macro or an inline
>   function. However, if you use it in an inlined function and pass an
>   argument of the function as the argument to the built-in, GCC never
>   returns 1 when you call the inline function with a string constant or
>   compound literal (see Compound Literals) and does not return 1 when
>   you pass a constant numeric value to the inline function **unless you
>   specify the -O option**.
>
> (emphasis mine).


Yes, I know. This difference was surprising for me and lots of other
people as well. But this is a fundamental position for clang and is
related to some implementation choices. Namely, C/C++ frontend needs
to know values of compile-time const expressions in order to verify
correctness and generate middle-end representation. But for gcc's
meaning of __builtin_constant_p, its value only becomes known deep
inside of middle-end. Which kinda creates a cycle. In gcc it's all
somehow mixed together (front-end/middle-end) and somehow works. Can't
possibly work for clang with strict separation between front-end and
middle-end.

I proposed to introduce another builtin that returns a value that is
constant from optimizer point of view (e.g. it can eliminate dead code
on branches), but is not constant from language/front-end point of
view (e.g. you can't declare a stack array using the value as size).
It should do in such cases and should be implementable in clang. But
we don't have it yet, and again it's not __builtin_constant_p, because
gcc's __builtin_constant_p returns a compile-time constant. So we need
to deal with this situation somehow.


>> I've installed clang-3.9 (the closest version to yours my distribution
>> gives me) and still got the same error with it. I would expect that
>> 4.0 should give the same result as well... Are you sure you enabled
>> KVM/intel/amd? (yes, I know you are maintaining KVM code :))
>
> Heh, I don't know about Radim but "^Rmake" goes straight to "make
> arch/x86/kvm/" for me. :)
>
> Paolo



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux