MIME-Version: 1.0
In-Reply-To: <5900628c-222c-fa71-ae11-00ce611c56dd@redhat.com>
References: <CACT4Y+boDPGU_02AhzUCCHfHdPaviMd5sTJTuV5RuYNor4wRpg@mail.gmail.com>
 <20170912151851.GA24313@flask> <CACT4Y+bGG1pRqM_ww77PHirv_teD+mBVcXd1K=RPWRzyWpmm7g@mail.gmail.com>
 <CACT4Y+Z7tsX-0bFUnPOpQHMrnjZvN0HX4aD5L6Xw7u4DZ_1Oqg@mail.gmail.com> <5900628c-222c-fa71-ae11-00ce611c56dd@redhat.com>
From: Dmitry Vyukov <dvyukov@google.com>
Date: Tue, 12 Sep 2017 18:16:27 +0200
Message-ID: <CACT4Y+bRNVv9gcTcK+XCZROawa5pmiDBpb9fAQUhHm1Vej5Tfw@mail.gmail.com>
Subject: Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        David Hildenbrand <david@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>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3335
Lines: 87

On Tue, Sep 12, 2017 at 6:03 PM, Paolo Bonzini <pbonzini@redhat.com> 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