[PATCH 0/2] tcg/tci: Fix Clang build

[PATCH 0/2] tcg/tci: Fix Clang build

[Philippe Mathieu-Daudé]

Fix the build failure reported by Wataru Ashihara on [*]
and add a CI test to catch future problems.

[*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg771326.html

Philippe Mathieu-Daudé (2):
  tcg: Mark more tcg_out*() functions with attribute 'unused'
  gitlab-ci: Add a job building TCI with Clang

 tcg/tcg.c      | 30 +++++++++++++++++++++---------
 .gitlab-ci.yml | 22 ++++++++++++++++++++--
 2 files changed, 41 insertions(+), 11 deletions(-)

-- 
2.26.2

[PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused'

[Philippe Mathieu-Daudé]

The tcg_out* functions are utility routines that may or
may not be used by a particular backend. Similarly to commit
4196dca63b8, mark them with the 'unused' attribute to suppress
spurious warnings if they aren't used.

This fixes the build with --enable-tcg-interpreter:

  [98/151] Compiling C object libqemu-arm-softmmu.fa.p/tcg_tcg.c.o
  FAILED: libqemu-arm-softmmu.fa.p/tcg_tcg.c.o
  clang [...] -o libqemu-arm-softmmu.fa.p/tcg_tcg.c.o -c ../tcg/tcg.c
  ../tcg/tcg.c:136:20: error: unused function 'tcg_out_dupi_vec' [-Werror,-Wunused-function]

Reported-by: Wataru Ashihara <wataash@wataash.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tcg/tcg.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 472bf1755bf..a7fc2043cbf 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -123,24 +123,36 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, unsigned vecl,
                            unsigned vece, const TCGArg *args,
                            const int *const_args);
 #else
-static inline bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
-                                   TCGReg dst, TCGReg src)
+static __attribute__((unused)) inline bool tcg_out_dup_vec(TCGContext *s,
+                                                           TCGType type,
+                                                           unsigned vece,
+                                                           TCGReg dst,
+                                                           TCGReg src)
 {
     g_assert_not_reached();
 }
-static inline bool tcg_out_dupm_vec(TCGContext *s, TCGType type, unsigned vece,
-                                    TCGReg dst, TCGReg base, intptr_t offset)
+static __attribute__((unused)) inline bool tcg_out_dupm_vec(TCGContext *s,
+                                                            TCGType type,
+                                                            unsigned vece,
+                                                            TCGReg dst,
+                                                            TCGReg base,
+                                                            intptr_t offset)
 {
     g_assert_not_reached();
 }
-static inline void tcg_out_dupi_vec(TCGContext *s, TCGType type,
-                                    TCGReg dst, tcg_target_long arg)
+static __attribute__((unused)) inline void tcg_out_dupi_vec(TCGContext *s,
+                                                            TCGType type,
+                                                            TCGReg dst,
+                                                            tcg_target_long arg)
 {
     g_assert_not_reached();
 }
-static inline void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, unsigned vecl,
-                                  unsigned vece, const TCGArg *args,
-                                  const int *const_args)
+static __attribute__((unused)) inline void tcg_out_vec_op(TCGContext *s,
+                                                          TCGOpcode opc,
+                                                          unsigned vecl,
+                                                          unsigned vece,
+                                                          const TCGArg *args,
+                                                          const int *const_args)
 {
     g_assert_not_reached();
 }
-- 
2.26.2

[RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Philippe Mathieu-Daudé]

Split the current GCC build-tci job in 2, and use Clang
compiler in the new job.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC in case someone have better idea to optimize can respin this patch.

 .gitlab-ci.yml | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 01c9e46410d..9053161793f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -397,12 +397,12 @@ build-oss-fuzz:
     # Unrelated to fuzzer: run some tests with -fsanitize=address
     - cd build-oss-fuzz && make check-qtest-i386 check-unit
 
-build-tci:
+build-tci-gcc:
   <<: *native_build_job_definition
   variables:
     IMAGE: fedora
   script:
-    - TARGETS="aarch64 alpha arm hppa m68k microblaze moxie ppc64 s390x x86_64"
+    - TARGETS="aarch64 alpha arm hppa x86_64"
     - mkdir build
     - cd build
     - ../configure --enable-tcg-interpreter
@@ -416,6 +416,24 @@ build-tci:
         ./tests/qtest/cdrom-test || exit 1 ;
       done
     - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
+
+build-tci-clang:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: fedora
+  script:
+    - TARGETS="m68k microblaze moxie ppc64 s390x"
+    - mkdir build
+    - cd build
+    - ../configure --enable-tcg-interpreter --cc=clang --cxx=clang++
+        --target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu '; done)" || { cat config.log meson-logs/meson-log.txt && exit 1; }
+    - make -j"$JOBS"
+    - make tests/qtest/boot-serial-test tests/qtest/cdrom-test tests/qtest/pxe-test
+    - for tg in $TARGETS ; do
+        export QTEST_QEMU_BINARY="./qemu-system-${tg}" ;
+        ./tests/qtest/boot-serial-test || exit 1 ;
+        ./tests/qtest/cdrom-test || exit 1 ;
+      done
     - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
 
 # Alternate coroutines implementations are only really of interest to KVM users
-- 
2.26.2

Re: [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused'

[Richard Henderson]

On 1/10/21 6:27 AM, Philippe Mathieu-Daudé wrote:
> The tcg_out* functions are utility routines that may or
> may not be used by a particular backend. Similarly to commit
> 4196dca63b8, mark them with the 'unused' attribute to suppress
> spurious warnings if they aren't used.
> 
> This fixes the build with --enable-tcg-interpreter:
> 
>   [98/151] Compiling C object libqemu-arm-softmmu.fa.p/tcg_tcg.c.o
>   FAILED: libqemu-arm-softmmu.fa.p/tcg_tcg.c.o
>   clang [...] -o libqemu-arm-softmmu.fa.p/tcg_tcg.c.o -c ../tcg/tcg.c
>   ../tcg/tcg.c:136:20: error: unused function 'tcg_out_dupi_vec' [-Werror,-Wunused-function]
> 
> Reported-by: Wataru Ashihara <wataash@wataash.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tcg/tcg.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)


This does too much to fix that Werror, as all of the other functions are
unconditionally used.

Alternately, I'll re-test and merge my tcg constant branch, which will make
tcg_out_dupi_vec also unconditionally used.  Then we don't need
__attribute__((unused)) at all.


r~

Re: [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused'

[Philippe Mathieu-Daudé]

On 1/10/21 6:51 PM, Richard Henderson wrote:
> On 1/10/21 6:27 AM, Philippe Mathieu-Daudé wrote:
>> The tcg_out* functions are utility routines that may or
>> may not be used by a particular backend. Similarly to commit
>> 4196dca63b8, mark them with the 'unused' attribute to suppress
>> spurious warnings if they aren't used.
>>
>> This fixes the build with --enable-tcg-interpreter:
>>
>>   [98/151] Compiling C object libqemu-arm-softmmu.fa.p/tcg_tcg.c.o
>>   FAILED: libqemu-arm-softmmu.fa.p/tcg_tcg.c.o
>>   clang [...] -o libqemu-arm-softmmu.fa.p/tcg_tcg.c.o -c ../tcg/tcg.c
>>   ../tcg/tcg.c:136:20: error: unused function 'tcg_out_dupi_vec' [-Werror,-Wunused-function]
>>
>> Reported-by: Wataru Ashihara <wataash@wataash.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tcg/tcg.c | 30 +++++++++++++++++++++---------
>>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> 
> This does too much to fix that Werror, as all of the other functions are
> unconditionally used.
> 
> Alternately, I'll re-test and merge my tcg constant branch, which will make
> tcg_out_dupi_vec also unconditionally used.  Then we don't need
> __attribute__((unused)) at all.

OK, better then.

Regards,

Phil.

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Thomas Huth]

On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
> Split the current GCC build-tci job in 2, and use Clang
> compiler in the new job.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC in case someone have better idea to optimize can respin this patch.
> 
>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)

I'm not quite sure whether we should go down this road ... if we wanted to 
have full test coverage for clang, we'd need to duplicate *all* jobs to run 
them once with gcc and once with clang. And that would be just overkill.

I think we already catch most clang-related problems with the clang jobs 
that we already have in our CI, so problems like the ones that you've tried 
to address here should be very, very rare. So I'd rather vote for not 
splitting the job here.

  Thomas

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Daniel P. Berrangé]

On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
> > Split the current GCC build-tci job in 2, and use Clang
> > compiler in the new job.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > RFC in case someone have better idea to optimize can respin this patch.
> > 
> >   .gitlab-ci.yml | 22 ++++++++++++++++++++--
> >   1 file changed, 20 insertions(+), 2 deletions(-)
> 
> I'm not quite sure whether we should go down this road ... if we wanted to
> have full test coverage for clang, we'd need to duplicate *all* jobs to run
> them once with gcc and once with clang. And that would be just overkill.
> 
> I think we already catch most clang-related problems with the clang jobs
> that we already have in our CI, so problems like the ones that you've tried
> to address here should be very, very rare. So I'd rather vote for not
> splitting the job here.

We can't possibly cope with the fully expanded matrix of what are
theoretically possible combinations. Thus I think we should be guided
by what is expected real world usage by platforms we target.

Essentially for any given distro we're testing on, our primary focus
should be to use the toolchain that distro will build QEMU with.

IOW, for Windows and Linux distros our primary focus should be GCC,
while for macOS, and *BSD, our focus should be CLang.

If there are other combinations that are known to hit bugs not covered
by the standard distro patterns above, we might add a few more jobs.
The latter should be the exception though, otherwise our number of
jobs will grow without bound.

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

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Philippe Mathieu-Daudé]

On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
> On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>> Split the current GCC build-tci job in 2, and use Clang
>>> compiler in the new job.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> RFC in case someone have better idea to optimize can respin this patch.
>>>
>>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> I'm not quite sure whether we should go down this road ... if we wanted to
>> have full test coverage for clang, we'd need to duplicate *all* jobs to run
>> them once with gcc and once with clang. And that would be just overkill.
>>
>> I think we already catch most clang-related problems with the clang jobs
>> that we already have in our CI, so problems like the ones that you've tried
>> to address here should be very, very rare. So I'd rather vote for not
>> splitting the job here.
> 
> We can't possibly cope with the fully expanded matrix of what are
> theoretically possible combinations. Thus I think we should be guided
> by what is expected real world usage by platforms we target.
> 
> Essentially for any given distro we're testing on, our primary focus
> should be to use the toolchain that distro will build QEMU with.
> 
> IOW, for Windows and Linux distros our primary focus should be GCC,
> while for macOS, and *BSD, our focus should be CLang.

Sounds good.

Do we need a TCI job on macOS then?

Thanks,

Phil.

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Daniel P. Berrangé]

On Thu, Jan 21, 2021 at 12:18:18PM +0100, Philippe Mathieu-Daudé wrote:
> On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
> > On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
> >> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
> >>> Split the current GCC build-tci job in 2, and use Clang
> >>> compiler in the new job.
> >>>
> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>> ---
> >>> RFC in case someone have better idea to optimize can respin this patch.
> >>>
> >>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
> >>>   1 file changed, 20 insertions(+), 2 deletions(-)
> >>
> >> I'm not quite sure whether we should go down this road ... if we wanted to
> >> have full test coverage for clang, we'd need to duplicate *all* jobs to run
> >> them once with gcc and once with clang. And that would be just overkill.
> >>
> >> I think we already catch most clang-related problems with the clang jobs
> >> that we already have in our CI, so problems like the ones that you've tried
> >> to address here should be very, very rare. So I'd rather vote for not
> >> splitting the job here.
> > 
> > We can't possibly cope with the fully expanded matrix of what are
> > theoretically possible combinations. Thus I think we should be guided
> > by what is expected real world usage by platforms we target.
> > 
> > Essentially for any given distro we're testing on, our primary focus
> > should be to use the toolchain that distro will build QEMU with.
> > 
> > IOW, for Windows and Linux distros our primary focus should be GCC,
> > while for macOS, and *BSD, our focus should be CLang.
> 
> Sounds good.
> 
> Do we need a TCI job on macOS then?

TCI is only relevant if there is no native TCG host impl.

macOS only targets aarch64 and x86_64, both of which have TCG, so there
is no reason to use TCI on macOS  AFAICT


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

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Philippe Mathieu-Daudé]

On 1/21/21 12:21 PM, Daniel P. Berrangé wrote:
> On Thu, Jan 21, 2021 at 12:18:18PM +0100, Philippe Mathieu-Daudé wrote:
>> On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
>>> On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>> compiler in the new job.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>> RFC in case someone have better idea to optimize can respin this patch.
>>>>>
>>>>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>>>
>>>> I'm not quite sure whether we should go down this road ... if we wanted to
>>>> have full test coverage for clang, we'd need to duplicate *all* jobs to run
>>>> them once with gcc and once with clang. And that would be just overkill.
>>>>
>>>> I think we already catch most clang-related problems with the clang jobs
>>>> that we already have in our CI, so problems like the ones that you've tried
>>>> to address here should be very, very rare. So I'd rather vote for not
>>>> splitting the job here.
>>>
>>> We can't possibly cope with the fully expanded matrix of what are
>>> theoretically possible combinations. Thus I think we should be guided
>>> by what is expected real world usage by platforms we target.
>>>
>>> Essentially for any given distro we're testing on, our primary focus
>>> should be to use the toolchain that distro will build QEMU with.
>>>
>>> IOW, for Windows and Linux distros our primary focus should be GCC,
>>> while for macOS, and *BSD, our focus should be CLang.
>>
>> Sounds good.
>>
>> Do we need a TCI job on macOS then?
> 
> TCI is only relevant if there is no native TCG host impl.
> 
> macOS only targets aarch64 and x86_64, both of which have TCG, so there
> is no reason to use TCI on macOS  AFAICT

Yes, fine by me, but Wataru Ashihara reported the bug... ¯\_(ツ)_/¯

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Daniel P. Berrangé]

On Thu, Jan 21, 2021 at 12:48:21PM +0100, Philippe Mathieu-Daudé wrote:
> On 1/21/21 12:21 PM, Daniel P. Berrangé wrote:
> > On Thu, Jan 21, 2021 at 12:18:18PM +0100, Philippe Mathieu-Daudé wrote:
> >> On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
> >>> On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
> >>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
> >>>>> Split the current GCC build-tci job in 2, and use Clang
> >>>>> compiler in the new job.
> >>>>>
> >>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>>>> ---
> >>>>> RFC in case someone have better idea to optimize can respin this patch.
> >>>>>
> >>>>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
> >>>>>   1 file changed, 20 insertions(+), 2 deletions(-)
> >>>>
> >>>> I'm not quite sure whether we should go down this road ... if we wanted to
> >>>> have full test coverage for clang, we'd need to duplicate *all* jobs to run
> >>>> them once with gcc and once with clang. And that would be just overkill.
> >>>>
> >>>> I think we already catch most clang-related problems with the clang jobs
> >>>> that we already have in our CI, so problems like the ones that you've tried
> >>>> to address here should be very, very rare. So I'd rather vote for not
> >>>> splitting the job here.
> >>>
> >>> We can't possibly cope with the fully expanded matrix of what are
> >>> theoretically possible combinations. Thus I think we should be guided
> >>> by what is expected real world usage by platforms we target.
> >>>
> >>> Essentially for any given distro we're testing on, our primary focus
> >>> should be to use the toolchain that distro will build QEMU with.
> >>>
> >>> IOW, for Windows and Linux distros our primary focus should be GCC,
> >>> while for macOS, and *BSD, our focus should be CLang.
> >>
> >> Sounds good.
> >>
> >> Do we need a TCI job on macOS then?
> > 
> > TCI is only relevant if there is no native TCG host impl.
> > 
> > macOS only targets aarch64 and x86_64, both of which have TCG, so there
> > is no reason to use TCI on macOS  AFAICT
> 
> Yes, fine by me, but Wataru Ashihara reported the bug... ¯\_(ツ)_/¯

It doesn't look like they were using macOS - the message suggests
Ubuntu host, and AFAIK, all Ubuntu architectures have support
for TCG, so using TCI shouldn't have been required in the first
place.

I guess we could benefit from a TCI job of some kind that uses
CLang on at least 1 platform, since none exists.

This does yet again open up the question of whether we should be
supporting TCI at all in this particular user's scenario though,
since both KVM and TCG are available on Ubuntu x86 hosts already.


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

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Philippe Mathieu-Daudé]

On 1/21/21 1:02 PM, Daniel P. Berrangé wrote:
> On Thu, Jan 21, 2021 at 12:48:21PM +0100, Philippe Mathieu-Daudé wrote:
>> On 1/21/21 12:21 PM, Daniel P. Berrangé wrote:
>>> On Thu, Jan 21, 2021 at 12:18:18PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
>>>>> On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
>>>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>>>> compiler in the new job.
>>>>>>>
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>> ---
>>>>>>> RFC in case someone have better idea to optimize can respin this patch.
>>>>>>>
>>>>>>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> I'm not quite sure whether we should go down this road ... if we wanted to
>>>>>> have full test coverage for clang, we'd need to duplicate *all* jobs to run
>>>>>> them once with gcc and once with clang. And that would be just overkill.
>>>>>>
>>>>>> I think we already catch most clang-related problems with the clang jobs
>>>>>> that we already have in our CI, so problems like the ones that you've tried
>>>>>> to address here should be very, very rare. So I'd rather vote for not
>>>>>> splitting the job here.
>>>>>
>>>>> We can't possibly cope with the fully expanded matrix of what are
>>>>> theoretically possible combinations. Thus I think we should be guided
>>>>> by what is expected real world usage by platforms we target.
>>>>>
>>>>> Essentially for any given distro we're testing on, our primary focus
>>>>> should be to use the toolchain that distro will build QEMU with.
>>>>>
>>>>> IOW, for Windows and Linux distros our primary focus should be GCC,
>>>>> while for macOS, and *BSD, our focus should be CLang.
>>>>
>>>> Sounds good.
>>>>
>>>> Do we need a TCI job on macOS then?
>>>
>>> TCI is only relevant if there is no native TCG host impl.
>>>
>>> macOS only targets aarch64 and x86_64, both of which have TCG, so there
>>> is no reason to use TCI on macOS  AFAICT
>>
>> Yes, fine by me, but Wataru Ashihara reported the bug... ¯\_(ツ)_/¯
> 
> It doesn't look like they were using macOS - the message suggests
> Ubuntu host, and AFAIK, all Ubuntu architectures have support
> for TCG, so using TCI shouldn't have been required in the first
> place.
> 
> I guess we could benefit from a TCI job of some kind that uses
> CLang on at least 1 platform, since none exists.
> 
> This does yet again open up the question of whether we should be
> supporting TCI at all in this particular user's scenario though,
> since both KVM and TCG are available on Ubuntu x86 hosts already.

I understand Stefan envisions other use cases for TCI, which is
why it is still maintained. See:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg461131.html

I agree with your previous comment:
> we should be guided by what is expected real world usage by
> platforms we target. Essentially for any given distro we're
> testing on, our primary focus should be to use the toolchain
> that distro will build QEMU with.

This rarely used config does not justify adding yet another CI job.

Thanks,

Phil.

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Wainer dos Santos Moschetta]

Hi,

On 1/21/21 7:08 AM, Thomas Huth wrote:
> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>> Split the current GCC build-tci job in 2, and use Clang
>> compiler in the new job.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> RFC in case someone have better idea to optimize can respin this patch.
>>
>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>
> I'm not quite sure whether we should go down this road ... if we 
> wanted to have full test coverage for clang, we'd need to duplicate 
> *all* jobs to run them once with gcc and once with clang. And that 
> would be just overkill.

I agree with Thomas.

>
>
> I think we already catch most clang-related problems with the clang 
> jobs that we already have in our CI, so problems like the ones that 
> you've tried to address here should be very, very rare. So I'd rather 
> vote for not splitting the job here.

We got only one clang job on GitLab CI...

   build-clang:
     <<: *native_build_job_definition
     variables:
       IMAGE: fedora
       CONFIGURE_ARGS: --cc=clang --cxx=clang++
       TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
         ppc-softmmu s390x-softmmu arm-linux-user
       MAKE_CHECK_ARGS: check

... and others on Travis:

   "Clang (user)"

   "Clang (main-softmmu)"

   "Clang (other-softmmu)"

   "[s390x] Clang (disable-tcg)"

So I've some questions:

  * Can we move those first three Travis jobs to Gitlab CI? (I can work 
on that)

  * Do you think they cover the most common problems with clang?

- Wainer

>
>
>  Thomas

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Daniel P. Berrangé]

On Thu, Jan 21, 2021 at 03:05:43PM -0300, Wainer dos Santos Moschetta wrote:
> Hi,
> 
> On 1/21/21 7:08 AM, Thomas Huth wrote:
> > On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
> > > Split the current GCC build-tci job in 2, and use Clang
> > > compiler in the new job.
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > ---
> > > RFC in case someone have better idea to optimize can respin this patch.
> > > 
> > >   .gitlab-ci.yml | 22 ++++++++++++++++++++--
> > >   1 file changed, 20 insertions(+), 2 deletions(-)
> > 
> > I'm not quite sure whether we should go down this road ... if we wanted
> > to have full test coverage for clang, we'd need to duplicate *all* jobs
> > to run them once with gcc and once with clang. And that would be just
> > overkill.
> 
> I agree with Thomas.
> 
> > 
> > 
> > I think we already catch most clang-related problems with the clang jobs
> > that we already have in our CI, so problems like the ones that you've
> > tried to address here should be very, very rare. So I'd rather vote for
> > not splitting the job here.
> 
> We got only one clang job on GitLab CI...
> 
>   build-clang:
>     <<: *native_build_job_definition
>     variables:
>       IMAGE: fedora
>       CONFIGURE_ARGS: --cc=clang --cxx=clang++
>       TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
>         ppc-softmmu s390x-softmmu arm-linux-user
>       MAKE_CHECK_ARGS: check
> 
> ... and others on Travis:
> 
>   "Clang (user)"
> 
>   "Clang (main-softmmu)"
> 
>   "Clang (other-softmmu)"

I guess these three overlap partially with the build-clang job.

>   "[s390x] Clang (disable-tcg)"

Don't forget the  Cirrus CI jobs for freebsd and macOS will
be using  CLang too.

> 
> So I've some questions:
> 
>  * Can we move those first three Travis jobs to Gitlab CI? (I can work on
> that)

Yeah, if we move those three travis jobs they can replace the existing
build-clang job. We don't neccesssarily need to keep them as three
separate jobs - that split was just due to the Travis time limits.
If a different split works better on GitLab we can do that.

>  * Do you think they cover the most common problems with clang?

Should do I think, especially in addition to the Cirrus CI jobs.


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

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Thomas Huth]

On 21/01/2021 19.13, Daniel P. Berrangé wrote:
> On Thu, Jan 21, 2021 at 03:05:43PM -0300, Wainer dos Santos Moschetta wrote:
>> Hi,
>>
>> On 1/21/21 7:08 AM, Thomas Huth wrote:
>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>> Split the current GCC build-tci job in 2, and use Clang
>>>> compiler in the new job.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>> RFC in case someone have better idea to optimize can respin this patch.
>>>>
>>>>    .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>    1 file changed, 20 insertions(+), 2 deletions(-)
>>>
>>> I'm not quite sure whether we should go down this road ... if we wanted
>>> to have full test coverage for clang, we'd need to duplicate *all* jobs
>>> to run them once with gcc and once with clang. And that would be just
>>> overkill.
>>
>> I agree with Thomas.
>>
>>>
>>>
>>> I think we already catch most clang-related problems with the clang jobs
>>> that we already have in our CI, so problems like the ones that you've
>>> tried to address here should be very, very rare. So I'd rather vote for
>>> not splitting the job here.
>>
>> We got only one clang job on GitLab CI...
>>
>>    build-clang:
>>      <<: *native_build_job_definition
>>      variables:
>>        IMAGE: fedora
>>        CONFIGURE_ARGS: --cc=clang --cxx=clang++
>>        TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
>>          ppc-softmmu s390x-softmmu arm-linux-user
>>        MAKE_CHECK_ARGS: check
>>
>> ... and others on Travis:
>>
>>    "Clang (user)"
>>
>>    "Clang (main-softmmu)"
>>
>>    "Clang (other-softmmu)"
> 
> I guess these three overlap partially with the build-clang job.
> 
>>    "[s390x] Clang (disable-tcg)"
> 
> Don't forget the  Cirrus CI jobs for freebsd and macOS will
> be using  CLang too.

Right... we should work towards getting cirrus-run into the QEMU-CI, too, to 
finally have these in the gitlab-ci dashboard, too.

>>
>> So I've some questions:
>>
>>   * Can we move those first three Travis jobs to Gitlab CI? (I can work on
>> that)
> 
> Yeah, if we move those three travis jobs they can replace the existing
> build-clang job. We don't neccesssarily need to keep them as three
> separate jobs - that split was just due to the Travis time limits.
> If a different split works better on GitLab we can do that.

Well, if we really want to increase the amount clang jobs, one of them 
should likely use TCI, as Phillippe suggested.

>>   * Do you think they cover the most common problems with clang?
> 
> Should do I think, especially in addition to the Cirrus CI jobs.

I concur.

  Thomas

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Wainer dos Santos Moschetta]

On 1/21/21 3:28 PM, Thomas Huth wrote:
> On 21/01/2021 19.13, Daniel P. Berrangé wrote:
>> On Thu, Jan 21, 2021 at 03:05:43PM -0300, Wainer dos Santos Moschetta 
>> wrote:
>>> Hi,
>>>
>>> On 1/21/21 7:08 AM, Thomas Huth wrote:
>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>> compiler in the new job.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>> RFC in case someone have better idea to optimize can respin this 
>>>>> patch.
>>>>>
>>>>>    .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>    1 file changed, 20 insertions(+), 2 deletions(-)
>>>>
>>>> I'm not quite sure whether we should go down this road ... if we 
>>>> wanted
>>>> to have full test coverage for clang, we'd need to duplicate *all* 
>>>> jobs
>>>> to run them once with gcc and once with clang. And that would be just
>>>> overkill.
>>>
>>> I agree with Thomas.
>>>
>>>>
>>>>
>>>> I think we already catch most clang-related problems with the clang 
>>>> jobs
>>>> that we already have in our CI, so problems like the ones that you've
>>>> tried to address here should be very, very rare. So I'd rather vote 
>>>> for
>>>> not splitting the job here.
>>>
>>> We got only one clang job on GitLab CI...
>>>
>>>    build-clang:
>>>      <<: *native_build_job_definition
>>>      variables:
>>>        IMAGE: fedora
>>>        CONFIGURE_ARGS: --cc=clang --cxx=clang++
>>>        TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
>>>          ppc-softmmu s390x-softmmu arm-linux-user
>>>        MAKE_CHECK_ARGS: check
>>>
>>> ... and others on Travis:
>>>
>>>    "Clang (user)"
>>>
>>>    "Clang (main-softmmu)"
>>>
>>>    "Clang (other-softmmu)"
>>
>> I guess these three overlap partially with the build-clang job.
>>
>>>    "[s390x] Clang (disable-tcg)"
>>
>> Don't forget the  Cirrus CI jobs for freebsd and macOS will
>> be using  CLang too.
>
> Right... we should work towards getting cirrus-run into the QEMU-CI, 
> too, to finally have these in the gitlab-ci dashboard, too.
>
>>>
>>> So I've some questions:
>>>
>>>   * Can we move those first three Travis jobs to Gitlab CI? (I can 
>>> work on
>>> that)
>>
>> Yeah, if we move those three travis jobs they can replace the existing
>> build-clang job. We don't neccesssarily need to keep them as three
>> separate jobs - that split was just due to the Travis time limits.
>> If a different split works better on GitLab we can do that.
>
> Well, if we really want to increase the amount clang jobs, one of them 
> should likely use TCI, as Phillippe suggested.
Ok, got it. I won't touch on those jobs.
>
>
>>>   * Do you think they cover the most common problems with clang?
>>
>> Should do I think, especially in addition to the Cirrus CI jobs.
>
> I concur.

Great. Thanks for the inputs.

- Wainer

>
>
>  Thomas

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Thomas Huth]

On 21/01/2021 21.46, Wainer dos Santos Moschetta wrote:
> 
> On 1/21/21 3:28 PM, Thomas Huth wrote:
>> On 21/01/2021 19.13, Daniel P. Berrangé wrote:
>>> On Thu, Jan 21, 2021 at 03:05:43PM -0300, Wainer dos Santos Moschetta wrote:
>>>> Hi,
>>>>
>>>> On 1/21/21 7:08 AM, Thomas Huth wrote:
>>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>>> compiler in the new job.
>>>>>>
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>> ---
>>>>>> RFC in case someone have better idea to optimize can respin this patch.
>>>>>>
>>>>>>    .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>>    1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>
>>>>> I'm not quite sure whether we should go down this road ... if we wanted
>>>>> to have full test coverage for clang, we'd need to duplicate *all* jobs
>>>>> to run them once with gcc and once with clang. And that would be just
>>>>> overkill.
>>>>
>>>> I agree with Thomas.
>>>>
>>>>>
>>>>>
>>>>> I think we already catch most clang-related problems with the clang jobs
>>>>> that we already have in our CI, so problems like the ones that you've
>>>>> tried to address here should be very, very rare. So I'd rather vote for
>>>>> not splitting the job here.
>>>>
>>>> We got only one clang job on GitLab CI...
>>>>
>>>>    build-clang:
>>>>      <<: *native_build_job_definition
>>>>      variables:
>>>>        IMAGE: fedora
>>>>        CONFIGURE_ARGS: --cc=clang --cxx=clang++
>>>>        TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
>>>>          ppc-softmmu s390x-softmmu arm-linux-user
>>>>        MAKE_CHECK_ARGS: check
>>>>
>>>> ... and others on Travis:
>>>>
>>>>    "Clang (user)"
>>>>
>>>>    "Clang (main-softmmu)"
>>>>
>>>>    "Clang (other-softmmu)"
>>>
>>> I guess these three overlap partially with the build-clang job.
>>>
>>>>    "[s390x] Clang (disable-tcg)"
>>>
>>> Don't forget the  Cirrus CI jobs for freebsd and macOS will
>>> be using  CLang too.
>>
>> Right... we should work towards getting cirrus-run into the QEMU-CI, too, 
>> to finally have these in the gitlab-ci dashboard, too.
>>
>>>>
>>>> So I've some questions:
>>>>
>>>>   * Can we move those first three Travis jobs to Gitlab CI? (I can work on
>>>> that)
>>>
>>> Yeah, if we move those three travis jobs they can replace the existing
>>> build-clang job. We don't neccesssarily need to keep them as three
>>> separate jobs - that split was just due to the Travis time limits.
>>> If a different split works better on GitLab we can do that.
>>
>> Well, if we really want to increase the amount clang jobs, one of them 
>> should likely use TCI, as Phillippe suggested.
> Ok, got it. I won't touch on those jobs.

I didn't meant my comment as a "no", but rather as a "needs further 
discussion first" ...

So here's my suggestion:

- Keep the current build-tci job as it is

- Add a build-clang-user job that compiles with clang and
   --disable-system

- Add a "build-clang-system 2" job that compiles with clang
   and --enable-tcg-interpreter and uses more or less the same
   targets as the "build-tci" job. Maybe add the avr-softmmu
   target here now, too, since it now also has a qtest with
   TCG (boot-serial-test)

- Rename the current "build-clang" job to "build-clang-system 1"
   and remove the arm-linux-user target and all the targets that
   are already part of the "build-clang-system 2" / "build-tci"
   from that job. Then add some other softmmu targets to that job
   (but take care that it does not exceed the 1h run time limit,
   so likely not all remaining targets can be added here)

- If we feel confident that we got enough test coverage there
   (together with the clang-based jobs on Cirrus-CI), finally
   remove the three x86 clang jobs from travis.yml.

What do you think? Could you work on such a patch (or patch series), Wainer?

  Thomas

Re: Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Wataru Ashihara]

On 2021/01/21 22:27, Philippe Mathieu-Daudé wrote:
> On 1/21/21 1:02 PM, Daniel P. Berrangé wrote:
>> On Thu, Jan 21, 2021 at 12:48:21PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 1/21/21 12:21 PM, Daniel P. Berrangé wrote:
>>>> On Thu, Jan 21, 2021 at 12:18:18PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
>>>>>> On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
>>>>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>>>>> compiler in the new job.
>>>>>>>>
>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>>> ---
>>>>>>>> RFC in case someone have better idea to optimize can respin this patch.
>>>>>>>>
>>>>>>>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>>>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> I'm not quite sure whether we should go down this road ... if we wanted to
>>>>>>> have full test coverage for clang, we'd need to duplicate *all* jobs to run
>>>>>>> them once with gcc and once with clang. And that would be just overkill.
>>>>>>>
>>>>>>> I think we already catch most clang-related problems with the clang jobs
>>>>>>> that we already have in our CI, so problems like the ones that you've tried
>>>>>>> to address here should be very, very rare. So I'd rather vote for not
>>>>>>> splitting the job here.
>>>>>>
>>>>>> We can't possibly cope with the fully expanded matrix of what are
>>>>>> theoretically possible combinations. Thus I think we should be guided
>>>>>> by what is expected real world usage by platforms we target.
>>>>>>
>>>>>> Essentially for any given distro we're testing on, our primary focus
>>>>>> should be to use the toolchain that distro will build QEMU with.
>>>>>>
>>>>>> IOW, for Windows and Linux distros our primary focus should be GCC,
>>>>>> while for macOS, and *BSD, our focus should be CLang.
>>>>>
>>>>> Sounds good.
>>>>>
>>>>> Do we need a TCI job on macOS then?
>>>>
>>>> TCI is only relevant if there is no native TCG host impl.
>>>>
>>>> macOS only targets aarch64 and x86_64, both of which have TCG, so there
>>>> is no reason to use TCI on macOS  AFAICT
>>>
>>> Yes, fine by me, but Wataru Ashihara reported the bug... ¯\_(ツ)_/¯
>>
>> It doesn't look like they were using macOS - the message suggests
>> Ubuntu host, and AFAIK, all Ubuntu architectures have support
>> for TCG, so using TCI shouldn't have been required in the first
>> place.
>>
>> I guess we could benefit from a TCI job of some kind that uses
>> CLang on at least 1 platform, since none exists.
>>
>> This does yet again open up the question of whether we should be
>> supporting TCI at all in this particular user's scenario though,
>> since both KVM and TCG are available on Ubuntu x86 hosts already.
> 
> I understand Stefan envisions other use cases for TCI, which is
> why it is still maintained. See:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg461131.html
> 
> I agree with your previous comment:
>> we should be guided by what is expected real world usage by
>> platforms we target. Essentially for any given distro we're
>> testing on, our primary focus should be to use the toolchain
>> that distro will build QEMU with.
> 
> This rarely used config does not justify adding yet another CI job.
> 
> Thanks,
> 
> Phil.
> 
> 

Actually I use TCI also on macOS. Like the use case quoted by Philippe,
there're even other reasons to use TCI:

1. Learning TCG ops.
2. Debugging QEMU with gdb. e.g. diagnose codegen or stepping into
   helper functions from tci.c:tcg_qemu_tb_exec().
3. Guest instruction tracing. TCI is faster than TCG or KVM when tracing
   the guest ops [1]. I guess qira is using TCI for this reason [2].

[1]: https://twitter.com/wata_ash/status/1352899988032942080
[2]: https://github.com/geohot/qira/blob/v1.3/tracers/qemu_build.sh#L55

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Stefan Weil]

Am 23.01.21 um 09:59 schrieb Wataru Ashihara:

> Actually I use TCI also on macOS. Like the use case quoted by Philippe,
> there're even other reasons to use TCI:
>
> 1. Learning TCG ops.
> 2. Debugging QEMU with gdb. e.g. diagnose codegen or stepping into
>     helper functions from tci.c:tcg_qemu_tb_exec().
> 3. Guest instruction tracing. TCI is faster than TCG or KVM when tracing
>     the guest ops [1]. I guess qira is using TCI for this reason [2].
>
> [1]: https://twitter.com/wata_ash/status/1352899988032942080
> [2]: https://github.com/geohot/qira/blob/v1.3/tracers/qemu_build.sh#L55


Yes, TCI can help a lot for debugging, especially also when porting TCG 
to a new host architecture.

If we had binaries which can switch from native to interpreted TCG, it 
could also be a reference implementation used for unit tests, comparing 
the results for each TCG opcode.

Using TCI with profiling like gprof is useful to count the frequency of 
the different TCG opcodes in practical scenarios and can be used to 
detect bottlenecks (and less frequent or unused opcodes) for native TCG, 
too.

Stefan

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Philippe Mathieu-Daudé]

On 1/23/21 11:26 AM, Stefan Weil wrote:
> Am 23.01.21 um 09:59 schrieb Wataru Ashihara:
> 
>> Actually I use TCI also on macOS. Like the use case quoted by Philippe,
>> there're even other reasons to use TCI:
>>
>> 1. Learning TCG ops.
>> 2. Debugging QEMU with gdb. e.g. diagnose codegen or stepping into
>>     helper functions from tci.c:tcg_qemu_tb_exec().
>> 3. Guest instruction tracing. TCI is faster than TCG or KVM when tracing
>>     the guest ops [1]. I guess qira is using TCI for this reason [2].
>>
>> [1]: https://twitter.com/wata_ash/status/1352899988032942080
>> [2]: https://github.com/geohot/qira/blob/v1.3/tracers/qemu_build.sh#L55
> 
> 
> Yes, TCI can help a lot for debugging, especially also when porting TCG
> to a new host architecture.

Indeed, Alistair used it to boostrap RISCV:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg552643.html

Worth citing a comment Peter mentioned at the end of this thread:
"the interpreter [...] only works with a subset of host OS calling
convention ABIs".
https://www.mail-archive.com/qemu-devel@nongnu.org/msg553077.html

> If we had binaries which can switch from native to interpreted TCG, it
> could also be a reference implementation used for unit tests, comparing
> the results for each TCG opcode.
> 
> Using TCI with profiling like gprof is useful to count the frequency of
> the different TCG opcodes in practical scenarios and can be used to
> detect bottlenecks (and less frequent or unused opcodes) for native TCG,
> too.
> 
> Stefan
> 
> 
> 

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Wainer dos Santos Moschetta]

Hi Thomas,

On 1/22/21 5:19 AM, Thomas Huth wrote:
> On 21/01/2021 21.46, Wainer dos Santos Moschetta wrote:
>>
>> On 1/21/21 3:28 PM, Thomas Huth wrote:
>>> On 21/01/2021 19.13, Daniel P. Berrangé wrote:
>>>> On Thu, Jan 21, 2021 at 03:05:43PM -0300, Wainer dos Santos 
>>>> Moschetta wrote:
>>>>> Hi,
>>>>>
>>>>> On 1/21/21 7:08 AM, Thomas Huth wrote:
>>>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>>>> compiler in the new job.
>>>>>>>
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>> ---
>>>>>>> RFC in case someone have better idea to optimize can respin this 
>>>>>>> patch.
>>>>>>>
>>>>>>>    .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>>>    1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> I'm not quite sure whether we should go down this road ... if we 
>>>>>> wanted
>>>>>> to have full test coverage for clang, we'd need to duplicate 
>>>>>> *all* jobs
>>>>>> to run them once with gcc and once with clang. And that would be 
>>>>>> just
>>>>>> overkill.
>>>>>
>>>>> I agree with Thomas.
>>>>>
>>>>>>
>>>>>>
>>>>>> I think we already catch most clang-related problems with the 
>>>>>> clang jobs
>>>>>> that we already have in our CI, so problems like the ones that 
>>>>>> you've
>>>>>> tried to address here should be very, very rare. So I'd rather 
>>>>>> vote for
>>>>>> not splitting the job here.
>>>>>
>>>>> We got only one clang job on GitLab CI...
>>>>>
>>>>>    build-clang:
>>>>>      <<: *native_build_job_definition
>>>>>      variables:
>>>>>        IMAGE: fedora
>>>>>        CONFIGURE_ARGS: --cc=clang --cxx=clang++
>>>>>        TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
>>>>>          ppc-softmmu s390x-softmmu arm-linux-user
>>>>>        MAKE_CHECK_ARGS: check
>>>>>
>>>>> ... and others on Travis:
>>>>>
>>>>>    "Clang (user)"
>>>>>
>>>>>    "Clang (main-softmmu)"
>>>>>
>>>>>    "Clang (other-softmmu)"
>>>>
>>>> I guess these three overlap partially with the build-clang job.
>>>>
>>>>>    "[s390x] Clang (disable-tcg)"
>>>>
>>>> Don't forget the  Cirrus CI jobs for freebsd and macOS will
>>>> be using  CLang too.
>>>
>>> Right... we should work towards getting cirrus-run into the QEMU-CI, 
>>> too, to finally have these in the gitlab-ci dashboard, too.
>>>
>>>>>
>>>>> So I've some questions:
>>>>>
>>>>>   * Can we move those first three Travis jobs to Gitlab CI? (I can 
>>>>> work on
>>>>> that)
>>>>
>>>> Yeah, if we move those three travis jobs they can replace the existing
>>>> build-clang job. We don't neccesssarily need to keep them as three
>>>> separate jobs - that split was just due to the Travis time limits.
>>>> If a different split works better on GitLab we can do that.
>>>
>>> Well, if we really want to increase the amount clang jobs, one of 
>>> them should likely use TCI, as Phillippe suggested.
>> Ok, got it. I won't touch on those jobs.
>
> I didn't meant my comment as a "no", but rather as a "needs further 
> discussion first" ...
>
> So here's my suggestion:
>
> - Keep the current build-tci job as it is
>
> - Add a build-clang-user job that compiles with clang and
>   --disable-system
>
> - Add a "build-clang-system 2" job that compiles with clang
>   and --enable-tcg-interpreter and uses more or less the same
>   targets as the "build-tci" job. Maybe add the avr-softmmu
>   target here now, too, since it now also has a qtest with
>   TCG (boot-serial-test)
>
> - Rename the current "build-clang" job to "build-clang-system 1"
>   and remove the arm-linux-user target and all the targets that
>   are already part of the "build-clang-system 2" / "build-tci"
>   from that job. Then add some other softmmu targets to that job
>   (but take care that it does not exceed the 1h run time limit,
>   so likely not all remaining targets can be added here)
>
> - If we feel confident that we got enough test coverage there
>   (together with the clang-based jobs on Cirrus-CI), finally
>   remove the three x86 clang jobs from travis.yml.
>
> What do you think? Could you work on such a patch (or patch series), 
> Wainer?

It looks reasonable to me. I will work on a patch series based on your 
suggestions so further discussion can be done on concrete changes.

Thanks!

- Wainer

>
>
>  Thomas

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Alex Bennée]

Wataru Ashihara <wataash@wataash.com> writes:

> On 2021/01/21 22:27, Philippe Mathieu-Daudé wrote:
>> On 1/21/21 1:02 PM, Daniel P. Berrangé wrote:
>>> On Thu, Jan 21, 2021 at 12:48:21PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 1/21/21 12:21 PM, Daniel P. Berrangé wrote:
>>>>> On Thu, Jan 21, 2021 at 12:18:18PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>> On 1/21/21 11:32 AM, Daniel P. Berrangé wrote:
>>>>>>> On Thu, Jan 21, 2021 at 11:08:29AM +0100, Thomas Huth wrote:
>>>>>>>> On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:
>>>>>>>>> Split the current GCC build-tci job in 2, and use Clang
>>>>>>>>> compiler in the new job.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>>>> ---
>>>>>>>>> RFC in case someone have better idea to optimize can respin this patch.
>>>>>>>>>
>>>>>>>>>   .gitlab-ci.yml | 22 ++++++++++++++++++++--
>>>>>>>>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> I'm not quite sure whether we should go down this road ... if we wanted to
>>>>>>>> have full test coverage for clang, we'd need to duplicate *all* jobs to run
>>>>>>>> them once with gcc and once with clang. And that would be just overkill.
>>>>>>>>
>>>>>>>> I think we already catch most clang-related problems with the clang jobs
>>>>>>>> that we already have in our CI, so problems like the ones that you've tried
>>>>>>>> to address here should be very, very rare. So I'd rather vote for not
>>>>>>>> splitting the job here.
>>>>>>>
>>>>>>> We can't possibly cope with the fully expanded matrix of what are
>>>>>>> theoretically possible combinations. Thus I think we should be guided
>>>>>>> by what is expected real world usage by platforms we target.
>>>>>>>
>>>>>>> Essentially for any given distro we're testing on, our primary focus
>>>>>>> should be to use the toolchain that distro will build QEMU with.
>>>>>>>
>>>>>>> IOW, for Windows and Linux distros our primary focus should be GCC,
>>>>>>> while for macOS, and *BSD, our focus should be CLang.
>>>>>>
>>>>>> Sounds good.
>>>>>>
>>>>>> Do we need a TCI job on macOS then?
>>>>>
>>>>> TCI is only relevant if there is no native TCG host impl.
>>>>>
>>>>> macOS only targets aarch64 and x86_64, both of which have TCG, so there
>>>>> is no reason to use TCI on macOS  AFAICT
>>>>
>>>> Yes, fine by me, but Wataru Ashihara reported the bug... ¯\_(ツ)_/¯
>>>
>>> It doesn't look like they were using macOS - the message suggests
>>> Ubuntu host, and AFAIK, all Ubuntu architectures have support
>>> for TCG, so using TCI shouldn't have been required in the first
>>> place.
>>>
>>> I guess we could benefit from a TCI job of some kind that uses
>>> CLang on at least 1 platform, since none exists.
>>>
>>> This does yet again open up the question of whether we should be
>>> supporting TCI at all in this particular user's scenario though,
>>> since both KVM and TCG are available on Ubuntu x86 hosts already.
>> 
>> I understand Stefan envisions other use cases for TCI, which is
>> why it is still maintained. See:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg461131.html
>> 
>> I agree with your previous comment:
>>> we should be guided by what is expected real world usage by
>>> platforms we target. Essentially for any given distro we're
>>> testing on, our primary focus should be to use the toolchain
>>> that distro will build QEMU with.
>> 
>> This rarely used config does not justify adding yet another CI job.
>> 
>> Thanks,
>> 
>> Phil.
>> 
>> 
>
> Actually I use TCI also on macOS. Like the use case quoted by Philippe,
> there're even other reasons to use TCI:
>
> 1. Learning TCG ops.

Except it's only a subset of ops. Really interesting newer ones using
the TCGv_vec types are entirely absent.

> 2. Debugging QEMU with gdb. e.g. diagnose codegen or stepping into
>    helper functions from tci.c:tcg_qemu_tb_exec().

I do this quite often with TCG so I'm curious as to what the difference
is here?

> 3. Guest instruction tracing. TCI is faster than TCG or KVM when tracing
>    the guest ops [1]. I guess qira is using TCI for this reason [2].

How are you doing instruction tracing with TCG? Using the plugin
interface?

I think there probably is a roll for a *guest* interpreter given the
amount of code that is translated only to be run once. However it would
be a fairly large undertaking.

> [1]: https://twitter.com/wata_ash/status/1352899988032942080
> [2]: https://github.com/geohot/qira/blob/v1.3/tracers/qemu_build.sh#L55


-- 
Alex Bennée

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Alex Bennée]

Stefan Weil <sw@weilnetz.de> writes:

> Am 23.01.21 um 09:59 schrieb Wataru Ashihara:
>
>> Actually I use TCI also on macOS. Like the use case quoted by Philippe,
>> there're even other reasons to use TCI:
>>
>> 1. Learning TCG ops.
>> 2. Debugging QEMU with gdb. e.g. diagnose codegen or stepping into
>>     helper functions from tci.c:tcg_qemu_tb_exec().
>> 3. Guest instruction tracing. TCI is faster than TCG or KVM when tracing
>>     the guest ops [1]. I guess qira is using TCI for this reason [2].
>>
>> [1]: https://twitter.com/wata_ash/status/1352899988032942080
>> [2]: https://github.com/geohot/qira/blob/v1.3/tracers/qemu_build.sh#L55
>
>
> Yes, TCI can help a lot for debugging, especially also when porting TCG 
> to a new host architecture.
>
> If we had binaries which can switch from native to interpreted TCG, it 
> could also be a reference implementation used for unit tests, comparing 
> the results for each TCG opcode.
>
> Using TCI with profiling like gprof is useful to count the frequency of 
> the different TCG opcodes in practical scenarios and can be used to 
> detect bottlenecks (and less frequent or unused opcodes) for native TCG, 
> too.

FWIW I had a bunch of JIT profiling changes that exposed the TCG op
counts via the JIT profiler.  I think I even enabled the op counting by
default because it was fairly lightweight to add.

IOW I think more introspection can be brought into the core TCG code
rather than relying on TCI to achieve it.

-- 
Alex Bennée