| Message ID | 20210110162739.858087-3-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tcg/tci: Fix Clang build | expand |
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
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
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.
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
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... ¯\_(ツ)_/¯
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
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.
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
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
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
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
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
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
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
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 > > >
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
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
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.
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
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(-)