Re: [PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary
On 1/20/21 6:35 PM, Paolo Bonzini wrote:
> On 20/01/21 18:02, Philippe Mathieu-Daudé wrote:
>>>
>>> For patch 1, which files are not compiled with the patch that were
>>> compiled without?
>> softfloat.
>
> Really? I see this:
>
> specific_ss.add(when: 'CONFIG_TCG', if_true: files(
> 'fpu/softfloat.c',
> ...))
>
> Maybe
>
> -subdir('fp')
> +if 'CONFIG_TCG' in config_all
> + subdir('fp')
> +endif
>
> in tests/meson.build is enough?
Yes, 464 unnecessary objects removed :)
Re: [PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary
On 20/01/21 18:02, Philippe Mathieu-Daudé wrote:
For patch 1, which files are not compiled with the patch that were
compiled without?
softfloat.
Really? I see this:
specific_ss.add(when: 'CONFIG_TCG', if_true: files(
'fpu/softfloat.c',
...))
Maybe
-subdir('fp')
+if 'CONFIG_TCG' in config_all
+ subdir('fp')
+endif
in tests/meson.build is enough?
Paolo
Re: [PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary
On 1/20/21 5:46 PM, Paolo Bonzini wrote:
> On 20/01/21 16:19, Philippe Mathieu-Daudé wrote:
>> We do not need TCG and capstone all the times. In some
>> configuration we can leave them out.
>>
>> Last patch emit a warning when a user explicitly select an
>> accelerator that the build with not use.
>>
>> Philippe Mathieu-Daudé (3):
>> configure: Do not build TCG if not necessary
>> configure: Do not build/check for capstone when emulation is disabled
>> configure: Emit warning when accelerator requested but not needed
>>
>> configure | 37 -
>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>
>
> Nice, but I have some remarks on how the patches are done. :)
>
> For patch 1, which files are not compiled with the patch that were
> compiled without?
softfloat.
I'll mention and address Thomas and your's other comments.
Thanks,
Phil.
>
> For patch 2, I think it's enough to add "build_by_default: false" to
> libcapstone (and while you're at it, to libslirp and libfdt).
>
> Finally, I would prefer patch 3 to be done in Meson, right before the
> summary() call. You can use config_all to check, like
>
> if get_option('kvm').enabled() and not config_all.has_key('CONFIG_KVM')
>
> etc. This will also warn for e.g. --enable-kvm
> --target-list=sh4-softmmu, which could be considered an improvement over
> your patch.
>
> Paolo
>
Re: [PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary
On 20/01/21 16:19, Philippe Mathieu-Daudé wrote:
We do not need TCG and capstone all the times. In some
configuration we can leave them out.
Last patch emit a warning when a user explicitly select an
accelerator that the build with not use.
Philippe Mathieu-Daudé (3):
configure: Do not build TCG if not necessary
configure: Do not build/check for capstone when emulation is disabled
configure: Emit warning when accelerator requested but not needed
configure | 37 -
1 file changed, 36 insertions(+), 1 deletion(-)
Nice, but I have some remarks on how the patches are done. :)
For patch 1, which files are not compiled with the patch that were
compiled without?
For patch 2, I think it's enough to add "build_by_default: false" to
libcapstone (and while you're at it, to libslirp and libfdt).
Finally, I would prefer patch 3 to be done in Meson, right before the
summary() call. You can use config_all to check, like
if get_option('kvm').enabled() and not config_all.has_key('CONFIG_KVM')
etc. This will also warn for e.g. --enable-kvm
--target-list=sh4-softmmu, which could be considered an improvement over
your patch.
Paolo
[PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary
We do not need TCG and capstone all the times. In some configuration we can leave them out. Last patch emit a warning when a user explicitly select an accelerator that the build with not use. Philippe Mathieu-Daudé (3): configure: Do not build TCG if not necessary configure: Do not build/check for capstone when emulation is disabled configure: Emit warning when accelerator requested but not needed configure | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) -- 2.26.2
