On Tue, Mar 21, 2023 at 03:13:02PM +0100, Michal Prívozník wrote:
> On 3/21/23 11:46, Daniel P. Berrangé wrote:
> > On Mon, Mar 20, 2023 at 03:17:24PM +0100, Michal Prívozník wrote:
> >> On 3/16/23 15:51, Andrea Bolognani wrote:
> >>> On Wed, Mar 15, 2023 at 08:10:26PM +0100, Michal Privoznik wrote:
> >>>> We ran into so many clang bugs recently, that I'm strongly in favor of
> >>>> making it optional and aim on gcc only. Debugging these issues burns our
> >>>> time needlessly.
> >>>>
> >>> [...]
> >>>>
> >>>> After these patches, there is still one qemuxml2argvtest case failing,
> >>>> and it's simply because no matter how hard I try, I can't stop clang
> >>>> from optimizing the function.
> >>>>
> >>> [...]
> >>>>
> >>>> At this point, I think we can call clang broken and focus our time on
> >>>> developing libvirt. Not coming up with hacks around broken compilers.
> >>>
> >>> clang is the default compiler for FreeBSD and macOS, both of which
> >>> are officially supported platforms. Unless we are willing to change
> >>> that, ignoring clang is simply out of the question.
> >>
> >> I'm fine with doing that change. FreeBSD offers option to install gcc.
> >> And for macOS - the fact we compile there says absolutely nothing.
> >> Anybody willing to make libvirt work on macOS is more than welcome to
> >> post patches.
> >
> > clang is the system compiler for those platforms, and IMHO we should
> > be compatible with it. Just as Fedora was long unhappy with certain
> > apps trying to force use of clang, when Fedora's system compiler was
> > GCC, the opposite is reasonable for other platforms.
> >
> >>> A thought about VIR_OPTNONE. It seems to me that we'd want to apply
> >>> this to all the functions that currently are marked with G_NO_INLINE
> >>> for mocking purposes. So, wouldn't it make sense to instead have a
> >>> single VIR_MOCKABLE annotation that combines the two, and use that
> >>> everywhere?
> >>>
> >>
> >> The problem is, __attribute__((optnone)) works only when passed at
> >> function definition, while noinline can be passed at function
> >> definition.
> >
> > Is that a problem in fact ? We only need to suppress inlining when
> > the calling function is in the same compilation unit as the called
> > function. Likewise for this return value optimization. So if 'noinline'
> > annotation was in the definition, rather than the declaration, it
> > would be fine for our purposes.
> >
> > I think it would make sense to have a VIR_MOCKABLE annotation that
> > covered 'noline', 'optnone' and 'noipa' attributes (the precise set
> > varying according to what compiler we target).
> >
> > If anything this is more appealing that having it in the .h file,
> > since this annotations are not intended to influence callers from
> > external compilation units.
> >
> > We would need to update our test program that validates the
> > existance of noinline for mocked functions to look in the .c
> > instead of .h
>
> The problem we're facing is not that functions we mock are optimized,
> but their callers are optimized in such way that our mocking is
> ineffective. IOW:
>
> int mockThisFunction() {
> ...
> }
>
> int someRegularNotMockedFunction() {
> ...
> mockThisFunction();
> ...
> }
>
> we would need to annotate someRegularNotMockedFunction() and leave
> mockThisFunction() be. This will get hairy pretty soon.
>
> And just for the record: the correct set of attributes that work for me
> are: optnone and used.
Hmm, i tried adding optnone to both, and it didn't work for me.
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 :|
