[PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary

[PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary

[Philippe Mathieu-Daudé]

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

[PATCH 1/3] configure: Do not build TCG if not necessary

[Philippe Mathieu-Daudé]

We don't want to check/build TCG when no system/user emulation
is requested. This is useful in particular when building only:
 - tools
 - documentation

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 9f016b06b54..012c527e3cd 100755
--- a/configure
+++ b/configure
@@ -337,7 +337,7 @@ linux_io_uring="$default_feature"
 cap_ng="auto"
 attr="auto"
 xfs="$default_feature"
-tcg="enabled"
+tcg="auto"
 membarrier="$default_feature"
 vhost_net="$default_feature"
 vhost_crypto="$default_feature"
@@ -2282,6 +2282,15 @@ case " $target_list " in
   ;;
 esac
 
+if [ "$tcg" = "auto" ]; then
+    # if neither system nor user emulation requested, no need to build TCG
+    if [ "$softmmu" = "no" ] && [ "$linux_user" = "no" ] && [ "$bsd_user" = "no" ]; then
+        tcg="disabled"
+    else
+        tcg="enabled"
+    fi
+fi
+
 feature_not_found() {
   feature=$1
   remedy=$2
-- 
2.26.2

[PATCH 2/3] configure: Do not build/check for capstone when emulation is disabled

[Philippe Mathieu-Daudé]

The capstone library is only used by system and user mode
emulation. When it is not required, do not check for it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/configure b/configure
index 012c527e3cd..a5159157a49 100755
--- a/configure
+++ b/configure
@@ -2291,6 +2291,11 @@ if [ "$tcg" = "auto" ]; then
     fi
 fi
 
+# if neither system nor user emulation requested, capstone is not needed
+if [ "$softmmu" = "no" ] && [ "$linux_user" = "no" ] && [ "$bsd_user" = "no" ]; then
+    capstone="disabled"
+fi
+
 feature_not_found() {
   feature=$1
   remedy=$2
-- 
2.26.2

[PATCH 3/3] configure: Emit warning when accelerator requested but not needed

[Philippe Mathieu-Daudé]

In some configurations it might be pointless to check and
compile accelerator code. Do not deselect the accelerator,
but emit a warning.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/configure b/configure
index a5159157a49..09e1cd8bfe6 100755
--- a/configure
+++ b/configure
@@ -5514,6 +5514,27 @@ if test $git_update = 'yes' ; then
     (cd "${source_path}" && GIT="$git" "./scripts/git-submodule.sh" update "$git_submodules")
 fi
 
+if [ "$softmmu" = "no" ]; then
+    if [ "$tcg" = "enabled" ]; then
+        echo "WARN: TCG accelerator selected but no target requires it"
+    fi
+    if [ "$kvm" = "enabled" ]; then
+        echo "WARN: KVM accelerator selected but no target requires it"
+    fi
+    if [ "$xen" = "enabled" ]; then
+        echo "WARN: Xen accelerator selected but no target requires it"
+    fi
+    if [ "$hax" = "enabled" ]; then
+        echo "WARN: HAX accelerator selected but no target requires it"
+    fi
+    if [ "$hvf" = "enabled" ]; then
+        echo "WARN: HVF accelerator selected but no target requires it"
+    fi
+    if [ "$whpx" = "enabled" ]; then
+        echo "WARN: WHPX accelerator selected but no target requires it"
+    fi
+fi
+
 config_host_mak="config-host.mak"
 
 echo "# Automatically generated by configure - do not modify" > $config_host_mak
-- 
2.26.2

Re: [PATCH 3/3] configure: Emit warning when accelerator requested but not needed

[Thomas Huth]

On 20/01/2021 16.19, Philippe Mathieu-Daudé wrote:
> In some configurations it might be pointless to check and
> compile accelerator code. Do not deselect the accelerator,
> but emit a warning.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   configure | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/configure b/configure
> index a5159157a49..09e1cd8bfe6 100755
> --- a/configure
> +++ b/configure
> @@ -5514,6 +5514,27 @@ if test $git_update = 'yes' ; then
>       (cd "${source_path}" && GIT="$git" "./scripts/git-submodule.sh" update "$git_submodules")
>   fi
>   
> +if [ "$softmmu" = "no" ]; then
> +    if [ "$tcg" = "enabled" ]; then
> +        echo "WARN: TCG accelerator selected but no target requires it"
> +    fi

What about linux-user? It needs TCG, but it can also be compiled with 
softmmu disabled, can't it?

  Thomas

Re: [PATCH 3/3] configure: Emit warning when accelerator requested but not needed

[Philippe Mathieu-Daudé]

On 1/20/21 5:33 PM, Thomas Huth wrote:
> On 20/01/2021 16.19, Philippe Mathieu-Daudé wrote:
>> In some configurations it might be pointless to check and
>> compile accelerator code. Do not deselect the accelerator,
>> but emit a warning.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   configure | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/configure b/configure
>> index a5159157a49..09e1cd8bfe6 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5514,6 +5514,27 @@ if test $git_update = 'yes' ; then
>>       (cd "${source_path}" && GIT="$git" "./scripts/git-submodule.sh"
>> update "$git_submodules")
>>   fi
>>   +if [ "$softmmu" = "no" ]; then
>> +    if [ "$tcg" = "enabled" ]; then
>> +        echo "WARN: TCG accelerator selected but no target requires it"
>> +    fi
> 
> What about linux-user? It needs TCG, but it can also be compiled with
> softmmu disabled, can't it?

Indeed, thanks for reviewing.

Re: [PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary

[Paolo Bonzini]

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

Re: [PATCH 0/3] configure: Do not build TCG or link with capstone if not necessary

[Philippe Mathieu-Daudé]

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

[Paolo Bonzini]

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

[Philippe Mathieu-Daudé]

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 :)