Re: [PATCH v2 24/30] meson: add common hw files

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/23/25 12:58, Richard Henderson wrote:
> On 3/20/25 15:29, Pierrick Bouvier wrote:
>> Those files will be compiled once per base architecture ("arm" in this
>> case), instead of being compiled for every variant/bitness of
>> architecture.
>>
>> We make sure to not include target cpu definitions (exec/cpu-defs.h) by
>> defining header guard directly. This way, a given compilation unit can
>> access a specific cpu definition, but not access to compile time defines
>> associated.
>>
>> Previous commits took care to clean up some headers to not rely on
>> cpu-defs.h content.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    meson.build | 37 ++++++++++++++++++++++++++++++++++++-
>>    1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/meson.build b/meson.build
>> index c21974020dd..994d3e5d536 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -3691,6 +3691,7 @@ hw_arch = {}
>>    target_arch = {}
>>    target_system_arch = {}
>>    target_user_arch = {}
>> +hw_common_arch = {}
>>    
>>    # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
>>    # that is filled in by qapi/.
>> @@ -4089,6 +4090,34 @@ common_all = static_library('common',
>>                                implicit_include_directories: false,
>>                                dependencies: common_ss.all_dependencies())
>>    
>> +# construct common libraries per base architecture
>> +hw_common_arch_libs = {}
>> +foreach target : target_dirs
>> +  config_target = config_target_mak[target]
>> +  target_base_arch = config_target['TARGET_BASE_ARCH']
>> +
>> +  # check if already generated
>> +  if target_base_arch in hw_common_arch_libs
>> +    continue
>> +  endif
>> +
>> +  if target_base_arch in hw_common_arch
>> +    target_inc = [include_directories('target' / target_base_arch)]
>> +    src = hw_common_arch[target_base_arch]
>> +    lib = static_library(
>> +      'hw_' + target_base_arch,
>> +      build_by_default: false,
>> +      sources: src.all_sources() + genh,
>> +      include_directories: common_user_inc + target_inc,
>> +      implicit_include_directories: false,
>> +      # prevent common code to access cpu compile time
>> +      # definition, but still allow access to cpu.h
>> +      c_args: ['-DCPU_DEFS_H', '-DCOMPILING_SYSTEM_VS_USER', '-DCONFIG_SOFTMMU'],
> 
> Oof.  This really seems like a hack, but it does work,
> and I'm not sure what else to suggest.
> 

Yes, it's the best (least-worst in reality) solution I found.

Initially I simply tried to add them to libsystem.
However, it has some problems:
- Impossible to link arch files only for concerned targets (or you need 
to add when: [TARGET_X] everywhere, which is not convenient).
- They need specific flags (most notably header guard -DCPU_DEFS_H, to 
ensure we don't rely on cpu compile time defines), which is only 
achievable through static lib hack already used in our build system. So 
another library needs to be declared.

> All the rest of the meson-foo looks ok, but a second eye couldn't hurt.
> 

If someone else has a better idea achieving the same result (maybe 
Paolo?), I would be happy to implement it.

> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~