On Tue, Mar 21, 2023 at 10:46:36AM +0000, 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: > > > 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). Empirically 'optnone' does *NOT* fix the problem we face. The optimizations about return value still get performed with 'optnone' present :-( After chasing links I found https://reviews.llvm.org/D28404#641077 where there is this nugget "optnone isn't *really* no optimizations: clearly this is true, but then neither is -O0. We run the always inliner, a couple of other passes, and we run several parts of the code generators optimizer. I understand why optnone deficiencies (ie, too many optimizations) might be frustrating, but having *more users* seems likely to make this *better*." Much of that ticket is debate of whether 'optnone' should be thue same as '-O0' There is also a blog post http://maskray.me/blog/2021-05-09-fno-semantic-interposition which confirms what we discovered "For ELF -fpic, Clang never suppresses interprocedural optimizations (including inlining) on default visibility external linkage definitions. So projects relying on blocked interprocedural optimizations have been broken for years. They only probably work recently by specifying -fsemantic-interposition." By 'default visibility' they basically mean any symbol which is not marked as 'weak', which is what we tried todo before and then reverted with: commit 407a281a8e2b6c5078ba1148535663ea64fd9314 Author: Daniel P. Berrangé <berrange@xxxxxxxxxx> Date: Wed Jul 12 11:07:17 2017 +0100 Revert "Prevent more compiler optimization of mockable functions" This reverts commit e4b980c853d2114b25fa805a84ea288384416221. Looking at the revert again though, I think we might not be out of luck. We had to revert because when we put stuff into .a files it got lost when linked to the final executable when it was marked weak. I think we might be able to work around this using linker flag -Wl,--whole-archive. This wasn't practical in 2017 as we still used automake+libtool and they had a habit of re-ordering such args. Now we're using meson though, it has a 'link_whole' option integrated which should do the right thing with -Wl,--whole-archive. This might let us re-try the weak function approach. 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 :|
