Re: [PATCH 1/6] meson: build target libraries with common dependencies

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

On 5/17/25 8:00 AM, Paolo Bonzini wrote:
> On 5/16/25 07:27, Pierrick Bouvier wrote:
>> @@ -4131,12 +4137,17 @@ common_all = static_library('common',
>>    hw_common_arch_libs = {}
>>    target_common_arch_libs = {}
>>    target_common_system_arch_libs = {}
>> -foreach target : target_dirs
>> +foreach target_base_arch, config_base_arch : config_base_arch_mak
>>      config_target = config_target_mak[target]
>> -  target_base_arch = config_target['TARGET_BASE_ARCH']
> 
> Each target_base_arch is now processed only once.  Therefore, all the
> "if target_base_arch not in ..." tests can be removed.
>

Yes, that's a good point, thanks.

>>      target_inc = [include_directories('target' / target_base_arch)]
>>      inc = [common_user_inc + target_inc]
> 
>>            sources: src.all_sources() + genh,
>>            include_directories: inc,
>>            c_args: target_system_c_args,
>> -        dependencies: src.all_dependencies())
>> +        dependencies: src.all_dependencies() + common_deps)
>>          hw_common_arch_libs += {target_base_arch: lib}
>>        endif
>>      endif
> 
> ...
> 
>> @@ -4179,7 +4190,7 @@ foreach target : target_dirs
>>            sources: src.all_sources() + genh,
>>            include_directories: inc,
>>            c_args: target_system_c_args,
>> -        dependencies: src.all_dependencies())
>> +        dependencies: src.all_dependencies() + common_deps)
>>          target_common_system_arch_libs += {target_base_arch: lib}
>>        endif
>>      endif
> 
> There is no need for two separate libraries, since hw_* and
> target_system_* use the same flags.  You can do something like
>
>     system_src = []
>     if target_base_arch in hw_common_arch
>       system_src += hw_common_arch[target_base_arch].all_sources()
>       system_deps += hw_common_arch[target_base_arch].all_dependencies()
>     endif
>     if target_base_arch in target_common_system_arch
>       system_src += target_common_system_arch[target_base_arch].all_sources()
>       system_deps +=
> target_common_system_arch[target_base_arch].all_dependencies()
>     endif
>     if system_src.length() > 0
>       ...
>     endif
> 
> to build the two arrays of sources and dependencies.
> 
> If you reduce the libraries from 3 to 2, you could call them 'common_' +
> target_base_arch and 'system_' + target_base_arch.  That's more similar
> to the existing libcommon and libsystem.
>

I hesitated to do it previously, so I'll merge them together.

> Paolo
> 

Thanks for the review,
Pierrick