[PATCH v3 0/4] meson: Try to clarify TCG / TCI options for new users

[PATCH v3 0/4] meson: Try to clarify TCG / TCI options for new users

[Philippe Mathieu-Daudé]

Since v2:
- Included Thomas suggestions

Some new users get confused between 'TCG' and 'TCI' and enable
TCI when TCG is better for they needs. Try to clarify it is
better to not use TCI when native backend is available.

Note, before Meson, warnings were summarized at the end of
./configure. Now they are displayed earlier, and likely
missed IMHO. No clue how to improve that :/

Based-on: <20210121095616.1471869-1-philmd@redhat.com>

Philippe Mathieu-Daudé (4):
  meson: Explicit TCG backend used
  meson: Warn when TCI is selected but TCG backend is available
  configure: Improve TCI feature description
  configure: Reword --enable-tcg-interpreter as --disable-native-tcg

 configure   |  5 +++--
 meson.build | 11 +++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.26.2

[PATCH v3 1/4] meson: Explicit TCG backend used

[Philippe Mathieu-Daudé]

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 meson.build | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 8535a83fb70..d5b76150e49 100644
--- a/meson.build
+++ b/meson.build
@@ -229,7 +229,7 @@
 if not get_option('tcg').disabled()
   if cpu not in supported_cpus
     if 'CONFIG_TCG_INTERPRETER' in config_host
-      warning('Unsupported CPU @0@, will use TCG with TCI (experimental)'.format(cpu))
+      warning('Unsupported CPU @0@, will use TCG with TCI (experimental and slow)'.format(cpu))
     else
       error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
     endif
@@ -2419,8 +2419,12 @@
 endif
 summary_info += {'TCG support':       config_all.has_key('CONFIG_TCG')}
 if config_all.has_key('CONFIG_TCG')
+  if config_host.has_key('CONFIG_TCG_INTERPRETER')
+    summary_info += {'TCG backend':   'TCG interpreter (experimental)'}
+  else
+    summary_info += {'TCG backend':   'native (@0@)'.format(cpu)}
+  endif
   summary_info += {'TCG debug enabled': config_host.has_key('CONFIG_DEBUG_TCG')}
-  summary_info += {'TCG interpreter':   config_host.has_key('CONFIG_TCG_INTERPRETER')}
 endif
 summary_info += {'target list':       ' '.join(target_dirs)}
 if have_system
-- 
2.26.2

[PATCH v3 2/4] meson: Warn when TCI is selected but TCG backend is available

[Philippe Mathieu-Daudé]

Some new users get confused with 'TCG' and 'TCI', and enable TCI
support expecting to enable TCG.

Emit a warning when native TCG backend is available on the
host architecture, mentioning this is a suboptimal configuration.

Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 meson.build | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meson.build b/meson.build
index d5b76150e49..d3df5fa3516 100644
--- a/meson.build
+++ b/meson.build
@@ -234,6 +234,9 @@
       error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
     endif
   endif
+  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in supported_cpus
+    warning('Experimental TCI requested while native TCG is available on @0@, suboptimal performance expected'.format(cpu))
+  endif
   accelerators += 'CONFIG_TCG'
   config_host += { 'CONFIG_TCG': 'y' }
 endif
-- 
2.26.2

[PATCH v3 3/4] configure: Improve TCI feature description

[Philippe Mathieu-Daudé]

Users might want to enable all features, without realizing some
features have negative effect. Mention the TCI feature is slow
and experimental, hoping it will be selected knowingly.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 9f016b06b54..48bd6f48d7a 100755
--- a/configure
+++ b/configure
@@ -1753,7 +1753,7 @@ Advanced options (experts only):
   --with-trace-file=NAME   Full PATH,NAME of file to store traces
                            Default:trace-<pid>
   --disable-slirp          disable SLIRP userspace network connectivity
-  --enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)
+  --enable-tcg-interpreter enable TCI (TCG with bytecode interpreter, experimental and slow)
   --enable-malloc-trim     enable libc malloc_trim() for memory optimization
   --oss-lib                path to OSS library
   --cpu=CPU                Build for host CPU [$cpu]
-- 
2.26.2

[PATCH v3 4/4] configure: Reword --enable-tcg-interpreter as --disable-native-tcg

[Philippe Mathieu-Daudé]

Users might want to enable all features, without realizing some
features have negative effect. Rename '--enable-tcg-interpreter'
as '--disable-native-tcg' to avoid user selecting this feature
without understanding it. '--enable-tcg-interpreter' is kept in
for backward compability with scripts.

Suggested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 48bd6f48d7a..5e5ff779a69 100755
--- a/configure
+++ b/configure
@@ -1121,7 +1121,8 @@ for opt do
   ;;
   --disable-tcg-interpreter) tcg_interpreter="no"
   ;;
-  --enable-tcg-interpreter) tcg_interpreter="yes"
+  --enable-tcg-interpreter) # backward compatibility
+  --disable-native-tcg) tcg_interpreter="yes"
   ;;
   --disable-cap-ng)  cap_ng="disabled"
   ;;
@@ -1753,7 +1754,7 @@ Advanced options (experts only):
   --with-trace-file=NAME   Full PATH,NAME of file to store traces
                            Default:trace-<pid>
   --disable-slirp          disable SLIRP userspace network connectivity
-  --enable-tcg-interpreter enable TCI (TCG with bytecode interpreter, experimental and slow)
+  --disable-native-tcg     enable TCI (TCG with bytecode interpreter, experimental and slow)
   --enable-malloc-trim     enable libc malloc_trim() for memory optimization
   --oss-lib                path to OSS library
   --cpu=CPU                Build for host CPU [$cpu]
-- 
2.26.2

Re: [PATCH v3 4/4] configure: Reword --enable-tcg-interpreter as --disable-native-tcg

[Paolo Bonzini]

On 22/01/21 14:30, Philippe Mathieu-Daudé wrote:
> Users might want to enable all features, without realizing some
> features have negative effect. Rename '--enable-tcg-interpreter'
> as '--disable-native-tcg' to avoid user selecting this feature
> without understanding it. '--enable-tcg-interpreter' is kept in
> for backward compability with scripts.
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   configure | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 48bd6f48d7a..5e5ff779a69 100755
> --- a/configure
> +++ b/configure
> @@ -1121,7 +1121,8 @@ for opt do
>     ;;
>     --disable-tcg-interpreter) tcg_interpreter="no"
>     ;;
> -  --enable-tcg-interpreter) tcg_interpreter="yes"
> +  --enable-tcg-interpreter) # backward compatibility

Do you really want to break the old option?

> +  --disable-native-tcg) tcg_interpreter="yes"
>     ;;
>     --disable-cap-ng)  cap_ng="disabled"
>     ;;
> @@ -1753,7 +1754,7 @@ Advanced options (experts only):
>     --with-trace-file=NAME   Full PATH,NAME of file to store traces
>                              Default:trace-<pid>
>     --disable-slirp          disable SLIRP userspace network connectivity
> -  --enable-tcg-interpreter enable TCI (TCG with bytecode interpreter, experimental and slow)
> +  --disable-native-tcg     enable TCI (TCG with bytecode interpreter, experimental and slow)
>     --enable-malloc-trim     enable libc malloc_trim() for memory optimization
>     --oss-lib                path to OSS library
>     --cpu=CPU                Build for host CPU [$cpu]
> 

The problem here is that for some CPUs there is no native TCG...  I 
mean, what's unclear in "exprimental and slow"?

Paolo

Re: [PATCH v3 0/4] meson: Try to clarify TCG / TCI options for new users

[Paolo Bonzini]

On 22/01/21 14:30, Philippe Mathieu-Daudé wrote:
> Since v2:
> - Included Thomas suggestions
> 
> Some new users get confused between 'TCG' and 'TCI' and enable
> TCI when TCG is better for they needs. Try to clarify it is
> better to not use TCI when native backend is available.
> 
> Note, before Meson, warnings were summarized at the end of
> ./configure. Now they are displayed earlier, and likely
> missed IMHO. No clue how to improve that :/
> 
> Based-on: <20210121095616.1471869-1-philmd@redhat.com>
> 
> Philippe Mathieu-Daudé (4):
>    meson: Explicit TCG backend used
>    meson: Warn when TCI is selected but TCG backend is available
>    configure: Improve TCI feature description
>    configure: Reword --enable-tcg-interpreter as --disable-native-tcg
> 
>   configure   |  5 +++--
>   meson.build | 11 +++++++++--
>   2 files changed, 12 insertions(+), 4 deletions(-)
> 

This will have to be reworked because the TCI option is moved to Meson 
in the pull request I have just sent, but patches 1-3 are good in concept.

Paolo

Re: [PATCH v3 4/4] configure: Reword --enable-tcg-interpreter as --disable-native-tcg

[Philippe Mathieu-Daudé]

On 1/23/21 7:13 PM, Paolo Bonzini wrote:
> On 22/01/21 14:30, Philippe Mathieu-Daudé wrote:
>> Users might want to enable all features, without realizing some
>> features have negative effect. Rename '--enable-tcg-interpreter'
>> as '--disable-native-tcg' to avoid user selecting this feature
>> without understanding it. '--enable-tcg-interpreter' is kept in
>> for backward compability with scripts.
>>
>> Suggested-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   configure | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 48bd6f48d7a..5e5ff779a69 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1121,7 +1121,8 @@ for opt do
>>     ;;
>>     --disable-tcg-interpreter) tcg_interpreter="no"
>>     ;;
>> -  --enable-tcg-interpreter) tcg_interpreter="yes"
>> +  --enable-tcg-interpreter) # backward compatibility
> 
> Do you really want to break the old option?

I am not breaking it, I'm keeping it, but not listed
in the help. Maybe you meant "not break the old option",
in that case you suggest me to add a warning such "this
option is deprecated and has been replaced by
--disable-native-tcg"?

> 
>> +  --disable-native-tcg) tcg_interpreter="yes"
>>     ;;
>>     --disable-cap-ng)  cap_ng="disabled"
>>     ;;
>> @@ -1753,7 +1754,7 @@ Advanced options (experts only):
>>     --with-trace-file=NAME   Full PATH,NAME of file to store traces
>>                              Default:trace-<pid>
>>     --disable-slirp          disable SLIRP userspace network connectivity
>> -  --enable-tcg-interpreter enable TCI (TCG with bytecode interpreter,
>> experimental and slow)
>> +  --disable-native-tcg     enable TCI (TCG with bytecode interpreter,
>> experimental and slow)
>>     --enable-malloc-trim     enable libc malloc_trim() for memory
>> optimization
>>     --oss-lib                path to OSS library
>>     --cpu=CPU                Build for host CPU [$cpu]
>>
> 
> The problem here is that for some CPUs there is no native TCG...  I
> mean, what's unclear in "exprimental and slow"?

OK, we can skip this patch then.

> 
> Paolo
> 

Re: [PATCH v3 2/4] meson: Warn when TCI is selected but TCG backend is available

[Daniel P. Berrangé]

On Fri, Jan 22, 2021 at 02:30:02PM +0100, Philippe Mathieu-Daudé wrote:
> Some new users get confused with 'TCG' and 'TCI', and enable TCI
> support expecting to enable TCG.
> 
> Emit a warning when native TCG backend is available on the
> host architecture, mentioning this is a suboptimal configuration.
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index d5b76150e49..d3df5fa3516 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -234,6 +234,9 @@
>        error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
>      endif
>    endif
> +  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in supported_cpus
> +    warning('Experimental TCI requested while native TCG is available on @0@, suboptimal performance expected'.format(cpu))
> +  endif

warning('Use of the TCG interpretor is not recommended on this host
         architecture. There is a native TCG execution backend available
         which provides substantially better performance and reliability. 
         It is strongly recommended to remove the --enable-tcg-interpreter 
         configuration option on this architecture to use the native
	 backend.')


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|