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

[PATCH v2 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 v2 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 v2 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 v2 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..71bdc523aa0 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 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 v2 3/4] configure: Improve TCI feature description

[Thomas Huth]

On 22/01/2021 11.58, Philippe Mathieu-Daudé wrote:
> 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..71bdc523aa0 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 TCG with bytecode interpreter (experimental and slow)

I'd prefer if we could keep the "TCI" in there ... I remember having grep'ed 
for "tci" in the help output in the past, so I think it would be good to 
keep the TLA here. Maybe just put "TCI, slow" in the parantheses and omit 
"experimental"?

  Thomas

[RFC PATCH v2 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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC so it can be discarded from the series

 configure | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 71bdc523aa0..5e56fa76499 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 TCG with bytecode interpreter (experimental and slow)
+  --disable-native-tcg     enable 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: [RFC PATCH v2 4/4] configure: Reword --enable-tcg-interpreter as --disable-native-tcg

[Thomas Huth]

On 22/01/2021 11.58, 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> RFC so it can be discarded from the series
> 
>   configure | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 71bdc523aa0..5e56fa76499 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 TCG with bytecode interpreter (experimental and slow)
> +  --disable-native-tcg     enable TCG with bytecode interpreter (experimental and slow)

The more I think about it, the more I like the idea.

Reviewed-by: Thomas Huth <thuth@redhat.com>