Re: [Qemu-devel] [PATCH for-2.7 00/49] NEED_CPU_H / cpu.h / hw/hw.h cleanups

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> 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.