All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: "Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Stefan Weil" <sw@weilnetz.de>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Wataru Ashihara" <wataash@wataash.com>,
	qemu-devel@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang
Date: Fri, 22 Jan 2021 09:19:49 +0100	[thread overview]
Message-ID: <87297994-4495-681a-5359-7807e537fa89@redhat.com> (raw)
In-Reply-To: <762a020e-43a6-9228-1a44-0d38d086a61f@redhat.com>

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



  reply	other threads:[~2021-01-22  8:22 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2021-01-26 13:51               ` Wainer dos Santos Moschetta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87297994-4495-681a-5359-7807e537fa89@redhat.com \
    --to=thuth@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=berrange@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=sw@weilnetz.de \
    --cc=wainersm@redhat.com \
    --cc=wataash@wataash.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.