[Top][All Lists]

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

Re: [PATCH 0/3] Atomic cleanup + clang-12 build fix

From: Cole Robinson
Subject: Re: [PATCH 0/3] Atomic cleanup + clang-12 build fix
Date: Tue, 13 Jul 2021 11:18:36 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 7/13/21 10:43 AM, Richard Henderson wrote:
> On 7/12/21 5:37 PM, Richard Henderson wrote:
>> On 7/12/21 2:30 PM, Cole Robinson wrote:
>>> On 7/12/21 11:59 AM, Richard Henderson wrote:
>>>> The first two patches are not strictly required, but they
>>>> were useful in tracking down the root problem here.
>>>> I understand the logic behind the clang-12 warning, but I think
>>>> it's a clear mistake that it should be enabled by default for a
>>>> target where alignment is not enforced by default.
>>>> I found over a dozen places where we would have to manually add
>>>> QEMU_ALIGNED(8) to uint64_t declarations in order to suppress
>>>> all of the instances.  IMO there's no point fighting this.
>>> I tested your patches, they seem to get rid of the warnings. The errors
>>> persist.
>>> FWIW here's my reproduce starting from fedora 34 x86_64 host:
>>> $ sudo mock --root fedora-35-i386 --install dnf --install dnf-utils
>>> --install fedora-packager --install clang
>>> $ sudo mock --root fedora-35-i386 --shell --enable-network
>>> # dnf builddep -y qemu
>>> # git clone https://github.com/qemu/qemu
>>> # cd qemu
>>> # CC=clang CXX=clang++ ./configure --disable-werror
>>> # make V=1
>> Ho hum.  So, the warnings are where clang has decided to insert calls
>> to libatomic.
>> So we either have to
>> (1) work around all of the places, which, unless we set up an i386
>> clang-12 builder will quickly bitrot, or
> Update: (1) is out.  There's a warning in cputlb.c vs a pointer that's
> known to be aligned, and it still fires.  I have filed a bug:
>   https://bugs.llvm.org/show_bug.cgi?id=51076
>> (2) write our own routines, compatible with libatomic, using cmpxchg8b
>> directly.  which requires no (extra) locking, and so is compatible
>> with the tcg jit output, or
>> (3) file a bug with clang, and document "use clang-11 and not clang-12".
> So, Cole, with respect to (3), is this just general regression testing
> that discovered this (in which case, yay) or is there some other reason
> clang is required?
> Assuming that (3) isn't really viable long term, I guess (2) is the only
> viable option.

I never tested building qemu with clang prior to this so no idea if it's
a regression.

There's some interest in using clang (eventually with cfi) to build the
Fedora qemu package,  so I gave it a test run. If this case is
problematic we could keep using gcc for it and clang for every other
arch, in the short/medium term.

Richard can you clarify, do you think the errors are a clang bug as
well, or strictly a qemu issue? If it's clang maybe I can get Red Hat
llvm devs to help


reply via email to

[Prev in Thread] Current Thread [Next in Thread]