Re: [PATCH v5 2/4] target/ppc: added ifdefs around TCG-only code

[Bruno Piazera Larsen]

On 25/05/2021 10:02, Philippe Mathieu-Daudé wrote:

> On 5/25/21 1:53 PM, Bruno Larsen (billionai) wrote:
> > excp_helper.c, mmu-hash64.c and mmu_helper.c have some function
> > declarations that are TCG-only, and couldn't be easily moved to a
> > TCG only file, so ifdefs were added around them.
> >
> > We also needed ifdefs around some header files because helper-proto.h
> > includes trace/generated-helpers.h, which is never created when building
> > without TCG, and cpu_ldst.h includes tcg/tcg.h, whose containing folder
> > is not included as a -iquote. As future cleanup, we could change the
> > part of the configuration script to add those.
> >
> > cpu_init.c also had a callback definition that is TCG only and could be
> > removed as part of a future cleanup (all the dump_statistics part is
> > almost never used and will become obsolete as we transition to using
> > decodetree).
> >
> > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> > ---
> >  target/ppc/cpu_init.c    |  2 ++
> >  target/ppc/excp_helper.c | 21 ++++++++++++++++++---
> >  target/ppc/mmu-hash64.c  | 11 ++++++++++-
> >  target/ppc/mmu_helper.c  | 16 ++++++++++++++--
> >  4 files changed, 44 insertions(+), 6 deletions(-)
> >       
> Please have a look at commit range 0a31c16c9ce..a2b0a27d33e
> for the MIPS convertion.
>
> >  #if !defined(CONFIG_USER_ONLY)
> > +#ifdef CONFIG_TCG
> >  void helper_store_msr(CPUPPCState *env, target_ulong val)
> >  {
> >       
> For example this one is similar to commit d60146a9389, you
> could simply move this function to tcg/sysemu/msr_helpers.c
> and modify the meson file, then when TCG is not available,
> the file isn't built, without having to use #ifdef'ry.
>

I can see what you mean, but I think the point was to not create separate files solely based on the accelerator type.

It's up to dgibson if we use that approach, but I agree that it could make the code quite a bit cleaner.

The next question would then be: should we go the whole 9 yards and add tcg/sysemu/* and tcg/linux-user/*, or can we just use tcg/* and rely on devs reading and understanding the meson.build file? I believe tcg/sysemu/* is going to be very empty (for now, only what is in mmu-hash64.c and is TCG-only, IIRC), so it sounds like a bit of an overkill, but I also see the argument for future-proofing.

>

--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer