* [PATCH 0/2] tcg/tci: Fix Clang build @ 2021-01-10 16:27 Philippe Mathieu-Daudé 2021-01-10 16:27 ` [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' Philippe Mathieu-Daudé 2021-01-10 16:27 ` [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang Philippe Mathieu-Daudé 0 siblings, 2 replies; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-10 16:27 UTC (permalink / raw) To: qemu-devel Cc: Thomas Huth, Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Wataru Ashihara, Alex Bennée 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' 2021-01-10 16:27 [PATCH 0/2] tcg/tci: Fix Clang build Philippe Mathieu-Daudé @ 2021-01-10 16:27 ` Philippe Mathieu-Daudé 2021-01-10 17:51 ` Richard Henderson 2021-01-10 16:27 ` [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang Philippe Mathieu-Daudé 1 sibling, 1 reply; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-10 16:27 UTC (permalink / raw) To: qemu-devel Cc: Thomas Huth, Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Wataru Ashihara, Alex Bennée 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' 2021-01-10 16:27 ` [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' Philippe Mathieu-Daudé @ 2021-01-10 17:51 ` Richard Henderson 2021-01-10 21:01 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 23+ messages in thread From: Richard Henderson @ 2021-01-10 17:51 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Thomas Huth, Stefan Weil, Wataru Ashihara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Alex Bennée 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~ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' 2021-01-10 17:51 ` Richard Henderson @ 2021-01-10 21:01 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-10 21:01 UTC (permalink / raw) To: Richard Henderson, qemu-devel Cc: Thomas Huth, Stefan Weil, Wataru Ashihara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Alex Bennée 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-10 16:27 [PATCH 0/2] tcg/tci: Fix Clang build Philippe Mathieu-Daudé 2021-01-10 16:27 ` [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' Philippe Mathieu-Daudé @ 2021-01-10 16:27 ` Philippe Mathieu-Daudé 2021-01-21 10:08 ` Thomas Huth 1 sibling, 1 reply; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-10 16:27 UTC (permalink / raw) To: qemu-devel Cc: Thomas Huth, Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé, Wataru Ashihara, Alex Bennée 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-10 16:27 ` [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang Philippe Mathieu-Daudé @ 2021-01-21 10:08 ` Thomas Huth 2021-01-21 10:32 ` Daniel P. Berrangé 2021-01-21 18:05 ` Wainer dos Santos Moschetta 0 siblings, 2 replies; 23+ messages in thread From: Thomas Huth @ 2021-01-21 10:08 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Stefan Weil, Richard Henderson, Wataru Ashihara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Alex Bennée 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 10:08 ` Thomas Huth @ 2021-01-21 10:32 ` Daniel P. Berrangé 2021-01-21 11:18 ` Philippe Mathieu-Daudé 2021-01-21 18:05 ` Wainer dos Santos Moschetta 1 sibling, 1 reply; 23+ messages in thread From: Daniel P. Berrangé @ 2021-01-21 10:32 UTC (permalink / raw) To: Thomas Huth Cc: Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Alex Bennée, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé 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 :| ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 10:32 ` Daniel P. Berrangé @ 2021-01-21 11:18 ` Philippe Mathieu-Daudé 2021-01-21 11:21 ` Daniel P. Berrangé 0 siblings, 1 reply; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-21 11:18 UTC (permalink / raw) To: Daniel P. Berrangé, Thomas Huth, Richard Henderson Cc: Stefan Weil, qemu-devel, Wataru Ashihara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Alex Bennée 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 11:18 ` Philippe Mathieu-Daudé @ 2021-01-21 11:21 ` Daniel P. Berrangé 2021-01-21 11:48 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 23+ messages in thread From: Daniel P. Berrangé @ 2021-01-21 11:21 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Thomas Huth, Stefan Weil, Richard Henderson, qemu-devel, Wataru Ashihara, Alex Bennée, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé 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 :| ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 11:21 ` Daniel P. Berrangé @ 2021-01-21 11:48 ` Philippe Mathieu-Daudé 2021-01-21 12:02 ` Daniel P. Berrangé 0 siblings, 1 reply; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-21 11:48 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Thomas Huth, Stefan Weil, Richard Henderson, qemu-devel, Wataru Ashihara, Alex Bennée, Wainer dos Santos Moschetta, 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... ¯\_(ツ)_/¯ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 11:48 ` Philippe Mathieu-Daudé @ 2021-01-21 12:02 ` Daniel P. Berrangé 2021-01-21 13:27 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 23+ messages in thread From: Daniel P. Berrangé @ 2021-01-21 12:02 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Thomas Huth, Stefan Weil, Richard Henderson, qemu-devel, Wataru Ashihara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Alex Bennée 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 :| ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 12:02 ` Daniel P. Berrangé @ 2021-01-21 13:27 ` Philippe Mathieu-Daudé 2021-01-23 8:59 ` Wataru Ashihara 0 siblings, 1 reply; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-21 13:27 UTC (permalink / raw) To: Daniel P. Berrangé, Stefan Weil Cc: Thomas Huth, Stefan Weil, Richard Henderson, qemu-devel, Wataru Ashihara, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, Alex Bennée 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 13:27 ` Philippe Mathieu-Daudé @ 2021-01-23 8:59 ` Wataru Ashihara 2021-01-23 10:26 ` Stefan Weil 2021-01-26 16:42 ` Alex Bennée 0 siblings, 2 replies; 23+ messages in thread From: Wataru Ashihara @ 2021-01-23 8:59 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Daniel P. Berrangé, Stefan Weil Cc: Thomas Huth, Philippe Mathieu-Daudé, Richard Henderson, qemu-devel, Wainer dos Santos Moschetta, Alex Bennée 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-23 8:59 ` Wataru Ashihara @ 2021-01-23 10:26 ` Stefan Weil 2021-01-23 13:31 ` Philippe Mathieu-Daudé 2021-01-26 17:08 ` Alex Bennée 2021-01-26 16:42 ` Alex Bennée 1 sibling, 2 replies; 23+ messages in thread From: Stefan Weil @ 2021-01-23 10:26 UTC (permalink / raw) To: Wataru Ashihara, Philippe Mathieu-Daudé, Daniel P. Berrangé Cc: Thomas Huth, Philippe Mathieu-Daudé, Richard Henderson, qemu-devel, Wainer dos Santos Moschetta, Alex Bennée 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-23 10:26 ` Stefan Weil @ 2021-01-23 13:31 ` Philippe Mathieu-Daudé 2021-01-26 17:08 ` Alex Bennée 1 sibling, 0 replies; 23+ messages in thread From: Philippe Mathieu-Daudé @ 2021-01-23 13:31 UTC (permalink / raw) To: Stefan Weil, Wataru Ashihara, Daniel P. Berrangé Cc: Thomas Huth, Philippe Mathieu-Daudé, Alistair Francis, Richard Henderson, qemu-devel, Wainer dos Santos Moschetta, Alex Bennée 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 > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-23 10:26 ` Stefan Weil 2021-01-23 13:31 ` Philippe Mathieu-Daudé @ 2021-01-26 17:08 ` Alex Bennée 1 sibling, 0 replies; 23+ messages in thread From: Alex Bennée @ 2021-01-26 17:08 UTC (permalink / raw) To: Stefan Weil Cc: Thomas Huth, Daniel P. Berrangé, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Wainer dos Santos Moschetta, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-23 8:59 ` Wataru Ashihara 2021-01-23 10:26 ` Stefan Weil @ 2021-01-26 16:42 ` Alex Bennée 1 sibling, 0 replies; 23+ messages in thread From: Alex Bennée @ 2021-01-26 16:42 UTC (permalink / raw) To: Wataru Ashihara Cc: Thomas Huth, Daniel P. Berrangé, Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wainer dos Santos Moschetta, qemu-devel, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 10:08 ` Thomas Huth 2021-01-21 10:32 ` Daniel P. Berrangé @ 2021-01-21 18:05 ` Wainer dos Santos Moschetta 2021-01-21 18:13 ` Daniel P. Berrangé 1 sibling, 1 reply; 23+ messages in thread From: Wainer dos Santos Moschetta @ 2021-01-21 18:05 UTC (permalink / raw) To: Thomas Huth, Philippe Mathieu-Daudé, qemu-devel Cc: Alex Bennée, Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 18:05 ` Wainer dos Santos Moschetta @ 2021-01-21 18:13 ` Daniel P. Berrangé 2021-01-21 18:28 ` Thomas Huth 0 siblings, 1 reply; 23+ messages in thread From: Daniel P. Berrangé @ 2021-01-21 18:13 UTC (permalink / raw) To: Wainer dos Santos Moschetta Cc: Thomas Huth, Stefan Weil, Philippe Mathieu-Daudé, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Alex Bennée 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 :| ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 18:13 ` Daniel P. Berrangé @ 2021-01-21 18:28 ` Thomas Huth 2021-01-21 20:46 ` Wainer dos Santos Moschetta 0 siblings, 1 reply; 23+ messages in thread From: Thomas Huth @ 2021-01-21 18:28 UTC (permalink / raw) To: Daniel P. Berrangé, Wainer dos Santos Moschetta Cc: Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Alex Bennée, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 18:28 ` Thomas Huth @ 2021-01-21 20:46 ` Wainer dos Santos Moschetta 2021-01-22 8:19 ` Thomas Huth 0 siblings, 1 reply; 23+ messages in thread From: Wainer dos Santos Moschetta @ 2021-01-21 20:46 UTC (permalink / raw) To: Thomas Huth, Daniel P. Berrangé Cc: Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Alex Bennée, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-21 20:46 ` Wainer dos Santos Moschetta @ 2021-01-22 8:19 ` Thomas Huth 2021-01-26 13:51 ` Wainer dos Santos Moschetta 0 siblings, 1 reply; 23+ messages in thread From: Thomas Huth @ 2021-01-22 8:19 UTC (permalink / raw) To: Wainer dos Santos Moschetta, Daniel P. Berrangé Cc: Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Alex Bennée, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang 2021-01-22 8:19 ` Thomas Huth @ 2021-01-26 13:51 ` Wainer dos Santos Moschetta 0 siblings, 0 replies; 23+ messages in thread From: Wainer dos Santos Moschetta @ 2021-01-26 13:51 UTC (permalink / raw) To: Thomas Huth, Daniel P. Berrangé Cc: Stefan Weil, Richard Henderson, Philippe Mathieu-Daudé, Wataru Ashihara, qemu-devel, Alex Bennée, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-01-26 17:11 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-10 16:27 [PATCH 0/2] tcg/tci: Fix Clang build Philippe Mathieu-Daudé 2021-01-10 16:27 ` [PATCH 1/2] tcg: Mark more tcg_out*() functions with attribute 'unused' Philippe Mathieu-Daudé 2021-01-10 17:51 ` Richard Henderson 2021-01-10 21:01 ` Philippe Mathieu-Daudé 2021-01-10 16:27 ` [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang Philippe Mathieu-Daudé 2021-01-21 10:08 ` Thomas Huth 2021-01-21 10:32 ` Daniel P. Berrangé 2021-01-21 11:18 ` Philippe Mathieu-Daudé 2021-01-21 11:21 ` Daniel P. Berrangé 2021-01-21 11:48 ` Philippe Mathieu-Daudé 2021-01-21 12:02 ` Daniel P. Berrangé 2021-01-21 13:27 ` Philippe Mathieu-Daudé 2021-01-23 8:59 ` Wataru Ashihara 2021-01-23 10:26 ` Stefan Weil 2021-01-23 13:31 ` Philippe Mathieu-Daudé 2021-01-26 17:08 ` Alex Bennée 2021-01-26 16:42 ` Alex Bennée 2021-01-21 18:05 ` Wainer dos Santos Moschetta 2021-01-21 18:13 ` Daniel P. Berrangé 2021-01-21 18:28 ` Thomas Huth 2021-01-21 20:46 ` Wainer dos Santos Moschetta 2021-01-22 8:19 ` Thomas Huth 2021-01-26 13:51 ` Wainer dos Santos Moschetta
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.