[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.7 00/49] NEED_CPU_H / cpu.h / hw/hw.h clea
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH for-2.7 00/49] NEED_CPU_H / cpu.h / hw/hw.h cleanups |
Date: |
Mon, 18 Apr 2016 16:07:21 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Paolo Bonzini <address@hidden> 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.
- [Qemu-devel] [PATCH 40/50] dma: do not depend on kvm_enabled(), (continued)
- [Qemu-devel] [PATCH 40/50] dma: do not depend on kvm_enabled(), Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 44/50] arm: move arm_log_exception into .c file, Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 45/50] mips: move CP0 functions out of cpu.h, Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 49/50] hw: remove pio_addr_t, Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 46/50] hw: explicitly include qemu/log.h, Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 47/50] exec: extract exec/tb-context.h, Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 50/50] hw: clean up hw/hw.h includes, Paolo Bonzini, 2016/04/08
- [Qemu-devel] [PATCH 48/50] cpu: move exec-all.h inclusion out of cpu.h, Paolo Bonzini, 2016/04/08
- Re: [Qemu-devel] [PATCH for-2.7 00/49] NEED_CPU_H / cpu.h / hw/hw.h cleanups, Markus Armbruster, 2016/04/18
- Re: [Qemu-devel] [PATCH for-2.7 00/49] NEED_CPU_H / cpu.h / hw/hw.h cleanups,
Markus Armbruster <=