| Message ID | 20210120151916.1167448-1-philmd@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | configure: Do not build TCG or link with capstone if not necessary | expand |
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
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 >
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
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 :)