On Mon, Nov 25, 2024 at 08:00:00AM +0100, Paolo Bonzini wrote: > On Sun, Nov 24, 2024 at 9:23 PM Pierrick Bouvier > <pierrick.bouv...@linaro.org> wrote: > > -mms-bitfields is already the (silent) gcc default on windows, to mimic > > MSVC behaviour. Yes, it would be preferable to use this default and > > gcc_struct attribute, but right now, it does not work with clang. So the > > whole point is to accept a compromise for this. > > I don't understand the strong pushback against clang support on windows. > > Because of a "theoretical" problem, that was proved here we don't have > > currently, we are stuck with gcc_struct attribute. > > I think you are unnecessarily pessimistic: > > * there is no need for a compromise, eliminating gcc_struct is the > best solution but it needs proof that it introduces no guest-visible > changes > > * there is no pushback against clang support, there is pushback > against asking for a change without understanding the problem
To expand on this... Bear in mind that when stuff breaks, users file bugs, and the QEMU maintainers are on the receiving end of user demands to investigate and fix the problem. These kind of issues have the potential to be very hard to debug and diagnose. Maintainer time is precious, which is why we're conservative at making decisions that expose us to potential long term negative consequences / increased workload. Compiling with flags that were known to be incompatible with our code historically is a reasonable thing to reject. It should not be unexpected that when a request to change this has a higher burden of proof that other proposals. > clang's lack of support for gcc_struct is stupid, but we have to work > around it and we can, without introducing potential ABI breaks. Just > verify that gcc_struct still has any effect. Initially I mentioned > checking sizeof() but actually it's possible to do the same using > debug info, similar to your scripts. IMHO we need to have confidence not only in the current state of the code, but also that we're not going to accidentally regresss it in the future. This is what the gcc_struct attribute gives us confidence in. If we can do an assert with a 'sizeof' check, that would give us similar, but I presume any sizeof checks need to be manually written to specify the expected size ? A manual check for pahole output by comparison only tells us about the current point in time of QEMU code. As an alternative is it practical for us to eliminate all bitfields from our structs ? IIUC, we already have the preference that we use the 'BIT(n)' and 'BIT_ULL(n)' macros for accessing bits from full sized int32/int64 fields which avoids the semantic layout problems. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
