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

[Bruno Piazera Larsen <bruno.larsen@eldorado.org.br>]

[-- Attachment #1: Type: text/plain, Size: 2605 bytes --]


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 
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

[-- Attachment #2: Type: text/html, Size: 3574 bytes --]