From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48499) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1as9pn-0002a1-El for qemu-devel@nongnu.org; Mon, 18 Apr 2016 10:07:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1as9pk-0008O6-M1 for qemu-devel@nongnu.org; Mon, 18 Apr 2016 10:07:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50168) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1as9pk-0008O1-Eg for qemu-devel@nongnu.org; Mon, 18 Apr 2016 10:07:24 -0400 From: Markus Armbruster References: <1460147350-7601-1-git-send-email-pbonzini@redhat.com> Date: Mon, 18 Apr 2016 16:07:21 +0200 In-Reply-To: <1460147350-7601-1-git-send-email-pbonzini@redhat.com> (Paolo Bonzini's message of "Fri, 8 Apr 2016 22:28:20 +0200") Message-ID: <87r3e3au3q.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH for-2.7 00/49] NEED_CPU_H / cpu.h / hw/hw.h cleanups List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, peter.maydell@linaro.org Paolo Bonzini writes: > (CCs only on cover letter due to huge series). > > I am sending this now because of vacation coming soon (yay!). > This series removes usage of NEED_CPU_H from several central > include files in QEMU, most notably hw/hw.h and qemu-common.h. > Definitions conditional on NEED_CPU_H remain only in disas/disas.h, > exec/gdbstub.h, exec/helper-head.h and exec/log.h. > > The interesting patches are interspersed with other miscellaenous > cleanups that I won't really dwell on in the cover letter. Most > of them are just making indirect inclusions explicit. > > Patches 5 to 27 make sure that target-independent code can access > QOM objects for the CPU through an opaque type. This is useful > because often target-independent code uses a target-specific header > file that happens to use pointers to ARMCPU* or similar. The > target-independent code itself does not use the pointed-to object, > but the very presenece of the ARMCPU* name means that all users of > that header have to bring in cpu.h. By providing the opaque type, > a much smaller API can be exposed to all these users in hw/. > > Patches 34 to 37 remove NEED_CPU_H from hw/hw.h, exec/memory.h > and exec/cpu-common.h. > > Patches 38 and 39 remove two nested inclusions from qemu-common.h. > This should make Markus's patch to remove unnecessary qemu-common.h > inclusions even more effective. > > Patches 42 and 43 disentangle qemu-common.h and cpu.h, so that all > users of the latter have to be explicit. This has the biggest > effect on reducing include pollution (the next offender is now > exec/cpu-common.h). > > Patches 46 to 50 remove more nested inclusions, and especially: > 1) the inclusion of the (TCG-specific) exec-all.h header from > cpu.h, so that non-TCG functions cannot anymore creep into > exec-all.h; 2) indirect qemu-common.h inclusion in hw/hw.h. > > At the end, hw/hw.h includes 13 fewer headers indirectly compared > to before when NEED_CPU_H is not defined, and 27 fewer headers > when NEED_CPU_H is defined. This was found with the script of > patch 1, which produces the following statistics: > > Compiled 3979 files | After: 4006 (nmi.o now built per target) > 3773 files include qemu-common.h | After: 3702 (-71) > 1658 files include hw/hw.h | After: 1589 (-69) > 3101 files include cpu.h | After: 2337 (-764, -25%!) > 3800 files include qapi-types.h | After: 3811 (+11, mostly from nmi.c) > 844 files include generated-tracers.h | After: 844 > 1270 files include qapi/error.h | After: 1297 (+27, from nmi.c) > 1996 files include block/aio.h | After: 1647 (-349, -18%) > 3544 files include qom/object.h | After: 3514 (-30) > 3451 files include exec/memory.h | After: 3540 (+89, from indirect inclusions) > 3840 files include fpu/softfloat.h | After: 3701 (-139) > 3783 files include qemu/bswap.h | After: 3644 (-139) > | > osdep.h: | After: (adds exec/poison.h) > lines bytes files source | lines bytes files source > 174 4944 3 QEMU | 226 5217 4 QEMU > 17460 440677 157 total | 17512 440950 158 total > | > qemu-common.h: | After: > lines bytes files source | lines bytes files source > 7037 160007 14 QEMU | 5919 132798 12 QEMU > 24323 595740 168 total | 23205 568531 166 total > | > hw/hw.h: | After: > lines bytes files source | lines bytes files source > 9714 228659 36 QEMU | 8458 201740 24 QEMU > 27052 665298 192 total | 25796 638379 180 total > | > target-i386/cpu.h: | After: > lines bytes files source | lines bytes files source > 11259 270041 41 QEMU | 10981 263615 39 QEMU > 28597 706680 197 total | 28319 700254 195 total > | > hw/hw.h + NEED_CPU_H: | After: > lines bytes files source | lines bytes files source > 12340 294661 50 QEMU | 8407 201467 23 QEMU > 29678 731300 206 total | 25745 638106 179 total > > The next objectives should be removing unnecessary inclusions from/of > qemu-common.h (or delete it altogether) and exec/cpu-common.h. I skimmed the whole series, and it all looks sane to me. As your stats show, there's more work to do. However, I believe your series takes care of some of the hairier parts. Very much appreaciated.