[llvm-dev] Proposal: Make the VE target official

[Simon Moll via llvm-dev]

Hi all,

NEC have been building up target support for SX-Aurora TSUBASA in LLVM
upstream - the VE target (Vector Engine) [1]. Today VE support in
upstream is sufficient for scalar code generation from C code with
target-specific vector intrinsics.

We propose the switch from 'experimental' backend status to 'official',
throwing into the ring:

* The staging builder (clang-ve-ninja) [2]. This builder builds and
checks LLVM+Clang and compiler-rt. The functional compiler-rt tests
run C code on a VE device.
We will extend this builder to test all other runtime libraries
(libcxxabi, libcxx, ..) for VE as continue upstreaming.
* LIT tests. VE has extensive lit tests (test/CodeGen/VE) for scalar
code generation and vector intrinsics.
* D113093 [3] has the changes to make all compiler-rt tests pass on VE.
Remaining failures are due to denormal support, alignment
requirements and subtle differences in syscalls.
* The downstream reference implementation (LLVM-VE [4]). This includes
compiler-rt,libcxxabi,libcxx and openmp target offloading and will be
the basis for upstreaming.

What do you say?

- Simon

[1] Vector Engine:
https://www.nec.com/en/global/solutions/hpc/sx/vector_engine.html
[2] clang-ve-ninja (recent run):
https://lab.llvm.org/staging/#/builders/24/builds/6594
[3] compiler-rt patch: https://reviews.llvm.org/D113093
[4] llvm-ve source: https://github.com/sx-aurora-dev/llvm-project.git

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Renato Golin via llvm-dev]

On Wed, 3 Nov 2021 at 12:52, Simon Moll <Simon...@emea.nec.com> wrote:

We propose the switch from 'experimental' backend status to 'official',

Hi Simon,

The VE community seems to have adopted all requirements laid out in the new targets' document[1], so I guess it's just a matter of ironing out the transition.

* The staging builder (clang-ve-ninja) [2].  This builder builds and
  checks LLVM+Clang and compiler-rt. The functional compiler-rt tests
  run C code on a VE device.

This bot seems red for a long time. One of the ideas that reflect a target to be stable is that its buildbot is "mostly" green throughout the warmup period (and hopefully just before moving out of experimental).

* D113093 [3] has the changes to make all compiler-rt tests pass on VE.
  Remaining failures are due to denormal support, alignment
  requirements and subtle differences in syscalls.

Will this make the bots green? It's ok to disable tests that fail for known reasons (or are irrelevant), but we shouldn't disable tests just to make the bots green to promote the target.

* The downstream reference implementation (LLVM-VE [4]).  This includes
  compiler-rt,libcxxabi,libcxx and openmp target offloading and will be
  the basis for upstreaming.

I looked at the branch and it seems to have a single merged patch that is not only huge, but has a lot of style changes to unrelated code (ex. clang-tools-extra).

It would be much simpler if you had a branch with the changes that you want to upstream into separated patches rebase onto LLVM's main branch.

cheers,

--renato

[Simon Moll via llvm-dev]

Hi Renato,

On Mon, 2021-11-08 at 10:09 +0000, Renato Golin wrote:
> On Wed, 3 Nov 2021 at 12:52, Simon Moll <Simon...@emea.nec.com>
> wrote:
> > We propose the switch from 'experimental' backend status to
> > 'official',
> >
>
>
> Hi Simon,
>
> The VE community seems to have adopted all requirements laid out in
> the new targets' document[1], so I guess it's just a matter of
> ironing out the transition.
>
> > * The staging builder (clang-ve-ninja) [2].  This builder builds
> > and
> >   checks LLVM+Clang and compiler-rt. The functional compiler-rt
> > tests
> >   run C code on a VE device.
> >
>
>
> This bot seems red for a long time. One of the ideas that reflect a
> target to be stable is that its buildbot is "mostly" green throughout
> the warmup period (and hopefully just before moving out of
> experimental).

If you look at the build logs of clang-ve-ninja, you will see that the
"check all" tests for LLVM+Clang have been passing for a while.
What's failing is compiler-rt and we have a patch for that.

>
> > * D113093 [3] has the changes to make all compiler-rt tests pass on
> > VE.
> >   Remaining failures are due to denormal support, alignment
> >   requirements and subtle differences in syscalls.
> >
>
>
> Will this make the bots green? It's ok to disable tests that fail for
> known reasons (or are irrelevant), but we shouldn't disable tests
> just to make the bots green to promote the target.

Yes, the compiler-rt tests are failing for well understood reasons
(documented in the patch - check-all on LLVM+Clang is green). The patch
will make compiler-rt pass on VE by accounting for those (no denorm
support, syscall differences).
We explicitly include compiler-rt testing (even though it is failing)
to have LLVM-compiled code running on the VE in CI.. this is not
something we'd do for slick optics.

>
> > * The downstream reference implementation (LLVM-VE [4]).  This
> > includes
> >   compiler-rt,libcxxabi,libcxx and openmp target offloading and
> > will
> > be
> >   the basis for upstreaming.
> >
>
>
> I looked at the branch and it seems to have a single merged patch
> that is not only huge, but has a lot of style changes to unrelated
> code (ex. clang-tools-extra).
>
> It would be much simpler if you had a branch with the changes that
> you want to upstream into separated patches rebase onto LLVM's main
> branch.

The github repo is for reference only. If you look at our upstream
patch history, you will see that we submit small patches with tests and
follow the review protocol.

>
> cheers,
> --renato
>

Thanks,
Simon

[Renato Golin via llvm-dev]

On Mon, 8 Nov 2021 at 14:56, Simon Moll <Simon...@emea.nec.com> wrote:

If you look at the build logs of clang-ve-ninja, you will see that the
"check all" tests for LLVM+Clang have been passing for a while.
What's failing is compiler-rt and we have a patch for that.

Right, what we mean by "green bots" is that there should be no conditions for the bot to be considered a success. 

Buildbots *must* not only test all known functionality that is expected to work, but they also must not be "red".

This is something that perhaps isn't clear on the new target section of the documents but it's the modus operandi for a long time.

If the bot is red, or turns red easily, it can't be relied upon to convey success in target testing, because you can't expect non-NEC developers to know what's good and what's not, or what should pass and what shouldn't.

It's the responsibility of the bot owner (and ultimately, the target's community), to make sure the bots accurately reflect the quality of the target.

Therefore, a (perhaps undocumented) item on the checklist before moving out of experimental is: the bots must test the target and they must be green and stable (weeks without crashing for spurious reasons).

In VE's case, looking at the earlier builds and seeing that "clang check" passes them all, should be enough to assert history, but before the target is built by everyone else, the bot must be green.

Yes, the compiler-rt tests are failing for well understood reasons
(documented in the patch - check-all on LLVM+Clang is green). The patch
will make compiler-rt pass on VE by accounting for those (no denorm
support, syscall differences).
We explicitly include compiler-rt testing (even though it is failing)
to have LLVM-compiled code running on the VE in CI.. this is not
something we'd do for slick optics.

Right, I've done the same thing when turning on the Arm back-end. I built enough buildbots that shown that the target was working on the basic level, then disabled the compiler-rt and test-suite that were not passing with specific bugzilla items for each one, and then with time, I fixed all of them and then all Arm bots were green.

In your case, no other bot (should? will?) build compiler-rt for VE, so this shouldn't hit other bots, which will start building VE once it builds by default.

But your buildbot will still be the *only* bot that build VE proper and uses hardware, so it will be the representative of the VE target.

If it continues red, and it later on problems start to appear in the LIT tests, then other developers will look at your bot, red for ages, and will likely infer that no one cares, and disable the broken test.

Overall, it's much easier if the main bot is green and all the disabled tests have bugzilla entries showing that you are working on it.

The github repo is for reference only. If you look at our upstream
patch history, you will see that we submit small patches with tests and
follow the review protocol.

I know, that's not what I meant.

My point is that it's really hard to use that branch for reference because of all of the other non-VE stuff that is there too, bundled in a single merge commit. 

cheers,

--renato

[Philip Reames via llvm-dev]

+1 to Renato's points.

One extra point on the build bot is that your cycle time appears to be about 30 minutes.  That's not unreasonable, but faster cycles are always better (i.e. shorter blame lists).  Any chance you can reduce that time via e.g. more hardware or build config tweaks (such as ccache)?  I don't mean to suggest this as a blocking item, simple as an area where improvement is possible.

Philip

[Simon Moll via llvm-dev]

On Mon, 2021-11-08 at 13:43 -0800, Philip Reames wrote:
> +1 to Renato's points.
> One extra point on the build bot is that your cycle time appears to
> be about 30 minutes.  That's not unreasonable, but faster cycles are
> always better (i.e. shorter blame lists).  Any chance you can reduce
> that time via e.g. more hardware or build config tweaks (such as
> ccache)?  I don't mean to suggest this as a blocking item, simple as
> an area where improvement is possible.

We should be able to bring that down. clang-ve-ninja currently builds
everything from scratch (and it's all static - i'd love to have working
shared component libraries for faster/incremental builds).

We are also considering a second, faster builder that only builds and
tests LLVM+Clang. That would be the canary for any issues with the VE
backend.

clang-ve-ninja would be the slow but thorough builder that includes all
supported runtimes and runs compiled code on the VE.

Thanks for shedding some light on the more implicit items on the
checklist.

Once D113093 is in, clang-ve-ninja is expected to be green.
We can call that the stable state - everything that's tested is
supposed to work and any red-ness implies breakage.

This may be a good moment to mention that the compiler-rt patch
disables tests that will never work on VE - there is no fp denormal
support, for example.

> >
> > Overall, it's much easier if the main bot is green and all the
> > disabled tests have bugzilla entries showing that you are working
> > on it.

Using bugzilla for this makes sense - evidently for bugs but also to
track/document features that aren't ready yet. Besides that, i'd like
to have a CI-approach for turning on features (in particular runtimes,
which tend to be less incremental than the backend work). I am thinking
the following:

With the compiler-rt patch clang-ve-ninja will be green. The coverage
of that bot defines what's officially supported for VE at any given
moment.

We add a new staging buildbot that builds everything clang-ve-ninja
does plus the yet-unsupported features that we are currently working
on.
Initially, that bot will be 'red' while the official one has to be kept
'green'. Once we are confident about the feature/runtime - both bots
are 'green' - we will make the official bot test that feature, thereby
declaring the feature official. The staging bot will turn to new
experimental features.

Coming back to Philip's point about slow turn arounds, one advantage of
the "ambitious twin" of clang-ve-ninja is that we could experiment with
speeding up the build without affecting the official bot.

Btw, we are planning to port all LLVM runtimes to VE more or less. The
delta between clang-ve-ninja and the staging twin will mostly be
runtimes.

> >
> >  
> > > The github repo is for reference only. If you look at our
> > > upstream
> > >  patch history, you will see that we submit small patches with
> > > tests and
> > >  follow the review protocol.
> > >
> >
> > I know, that's not what I meant.
> >
> > My point is that it's really hard to use that branch for reference
> > because of all of the other non-VE stuff that is there too, bundled
> > in a single merge commit. 
> >
> > cheers,
> > --renato

Thanks
- Simon

[Simon Moll via llvm-dev]

On Mon, 2021-11-08 at 15:52 +0000, Renato Golin wrote:
>
>
> > The github repo is for reference only. If you look at our upstream
> > patch history, you will see that we submit small patches with tests
> > and
> > follow the review protocol.
> >
>
>
> I know, that's not what I meant.
>
> My point is that it's really hard to use that branch for reference
> because of all of the other non-VE stuff that is there too, bundled
> in a single merge commit. 

We merge upstream LLVM every now and then. There were feature-heavy
merges in the past ('develop' is the main branch). There is not much we
can do about the merge commits in retrospect. Is this a requirement? If
so, what would you like us to do?

>
> cheers,
> --renato
- Simon

[Philip Reames via llvm-dev]

On 11/10/21 6:08 AM, Simon Moll wrote:
> On Mon, 2021-11-08 at 13:43 -0800, Philip Reames wrote:
>> +1 to Renato's points.
>> One extra point on the build bot is that your cycle time appears to
>> be about 30 minutes.  That's not unreasonable, but faster cycles are
>> always better (i.e. shorter blame lists).  Any chance you can reduce
>> that time via e.g. more hardware or build config tweaks (such as
>> ccache)?  I don't mean to suggest this as a blocking item, simple as
>> an area where improvement is possible.
> We should be able to bring that down. clang-ve-ninja currently builds
> everything from scratch (and it's all static - i'd love to have working
> shared component libraries for faster/incremental builds).
>
> We are also considering a second, faster builder that only builds and
> tests LLVM+Clang. That would be the canary for any issues with the VE
> backend.
>
> clang-ve-ninja would be the slow but thorough builder that includes all
> supported runtimes and runs compiled code on the VE.

This all sounds entirely reasonable.  With the one caveat that
incremental can be risky.  I've been told that ccache works almost as
well with less risk of build weirdness.  (I don't personal maintain a
bot, so this is hearsay.)

[Renato Golin via llvm-dev]

On Wed, 10 Nov 2021 at 14:10, Simon Moll <Simon...@emea.nec.com> wrote:

We should be able to bring that down. clang-ve-ninja currently builds
everything from scratch (and it's all static - i'd love to have working
shared component libraries for faster/incremental builds).

We are also considering a second, faster builder that only builds and
tests LLVM+Clang. That would be the canary for any issues with the VE
backend.

 

IIUC, your builder is an x86_64 machine cross testing on a VE target. 

If the canary builder doesn't run anything on VE hardware, then after VE is official, it won't be different than any other x86_64 builder building/testing VE.

Once D113093 is in, clang-ve-ninja is expected to be green.
We can call that the stable state - everything that's tested is
supposed to work and any red-ness implies breakage.

Excellent! Make sure once green that the bot be moved to the production server. Check with Galina to make sure you're on the right place.

This may be a good moment to mention that the compiler-rt patch
disables tests that will never work on VE - there is no fp denormal
support, for example.

Ah, right, so then permanently disabled is the right thing to do, no need for bugzilla entries.

We add a new staging buildbot that builds everything clang-ve-ninja
does plus the yet-unsupported features that we are currently working
on.

If you do that, make sure you add it to the staging server (or a private one). This bot cannot notify people of breakages (email, IRC, nothing).

Initially, that bot will be 'red' while the official one has to be kept
'green'. Once we are confident about the feature/runtime - both bots
are 'green' - we will make the official bot test that feature, thereby
declaring the feature official. The staging bot will turn to new
experimental features.

Once it's official, and green, move the bot to the production (noisy) server and create a new one for new features in the staging one.

Once the target is out of experimental, every buildbot that doesn't restrict the targets it builds (most bots) will build VE and run its LIT tests.

Because you're the VE code owner, if your target breaks other people's bots, developers will (hopefully) notify you, too.

So your strategy would have at the very least three (classes of) bots:

1. Fast "canary" bot, building only Clang and LLVM and running LIT tests (maybe you'll want to add some cross-VE testing here, too)

2. Slow complete bot, building everything that is supposed to be supported. This can be multiple bots with different configurations or one huge build.

3. A staging (silent) build with new stuff that the team is working on. This can also be different bots, totally up to the team, and doesn't even need to be public.

I did a similar division for Arm and we've been doing it since for Arm 32 and 64. Even if we use the same server for multiple bots, it's easier to debug breakages when we build less stuff.

cheers,

--renato

[Simon Moll via llvm-dev]

We'll look into ccache for this. Thanks!

>  Click
> https://www.mailcontrol.com/sr/VFUjQu9VG-3GX2PQPOmvUjXuKmsvWB_mC-pKFlvAVhldgTo7ZGdh_Iu_pQm_b2t2G0oARboojJDGKFQ3ec5RLg==
>   to report this email as spam.

[Simon Moll via llvm-dev]

In essense, we keep the current staging clang-ve-ninja as is and attach
a clone to the main buildbot. The clone is then the official bot and
the staging one is the one for work-in-progress features.

That's the plan. Given that other/fast bots will build the VE target
and run its lit tests, we may not need the canary though.

>
> I did a similar division for Arm and we've been doing it since for
> Arm 32 and 64. Even if we use the same server for multiple bots, it's
> easier to debug breakages when we build less stuff.

libunwind, compiler-rt, libcxxabi, libcxx really dependend on each
other. As long as build times are reasonable, we will keep it to full
stack testing.

- Simon

>
> cheers,
> --renato

[Simon Moll via llvm-dev]

Hi,

we've addressed all points raised on the proposal to make VE an
official target:

* Buildbot worker times are down to 7-8mins. We use ccache (thx!).
* We're set up to enable a worker reporting to the main buildbot
instance.
* The `clang-ve-ninja` buildbot is green and its testing represents
what is supported for VE.

We're ready to switch to official status. We're planning to proceed as
follows once D113247 and D113389 are accepted.

1. Connect the VE worker to main buildbot instance.
2. Add `VE` to the LLVM target list, thereby making it official
(D113247).
3. Update llvm-zorg to build VE as an official target and enable
buildbot reporting for VE (D113389).
4. Attentively follow all builders until the expected aftermath has
settled.

Only after this, we will unfreeze VE feature development and spin up
the staging worker for work-in-progress features.

- Simon

[Renato Golin via llvm-dev]

On Mon, 29 Nov 2021 at 08:32, Simon Moll <Simon...@emea.nec.com> wrote:

* Buildbot worker times are down to 7-8mins. We use ccache (thx!).
* We're set up to enable a worker reporting to the main buildbot
instance.
* The `clang-ve-ninja` buildbot is green and its testing represents
what is supported for VE.

Awesome!

1. Connect the VE worker to main buildbot instance.
2. Add `VE` to the LLVM target list, thereby making it official
(D113247).

This will trigger all bots that don't restrict targets to build VE, and you may get some failures here already.

I'll add a pause here and make sure none of the failures are VE related.

3. Update llvm-zorg to build VE as an official target and enable
buildbot reporting for VE (D113389).

I don't understand this step. Does that mean you want to add VE to other buildbots? Or just that your buildbot will be added without VE in step 1?

--renato

[Simon Moll via llvm-dev]

On Mon, 2021-11-29 at 10:36 +0000, Renato Golin wrote:
> On Mon, 29 Nov 2021 at 08:32, Simon Moll <Simon...@emea.nec.com>
> wrote:
> > * Buildbot worker times are down to 7-8mins. We use ccache (thx!).
> > * We're set up to enable a worker reporting to the main buildbot
> > instance.
> > * The `clang-ve-ninja` buildbot is green and its testing represents
> > what is supported for VE.
> >
>
>
> Awesome!
>
> > 1. Connect the VE worker to main buildbot instance.
> > 2. Add `VE` to the LLVM target list, thereby making it official
> > (D113247).
> >
>
>
> This will trigger all bots that don't restrict targets to build VE,
> and you may get some failures here already.

I see no harm in moving clang-ve-ninja to main buildbot while we are
watching the ramifications play out for all builders. Builders don't
interfere with one another. There will only be better coverage.

>
> I'll add a pause here and make sure none of the failures are VE
> related.
>
> > 3. Update llvm-zorg to build VE as an official target and enable
> > buildbot reporting for VE (D113389).
> >
>
>
> I don't understand this step. Does that mean you want to add VE to
> other buildbots? Or just that your buildbot will be added without VE
> in step 1?

This only matters for the clang-ve-ninja builder. The llvm-zorg patch
moves 'VE' from the experimental target to the targets_to_build cmake
option for clang-ve-ninja and enables reporting on failure. This will
not effect other bots.
I wouldn't normally do an llvm-zorg patch for this but since it is part
of the concerted effort to make VE official i felt a Diff was in order.

- Simon

[Renato Golin via llvm-dev]

On Mon, 29 Nov 2021 at 12:36, Simon Moll <Simon...@emea.nec.com> wrote:

> > 1. Connect the VE worker to main buildbot instance.
> > 2. Add `VE` to the LLVM target list, thereby making it official
> > (D113247).
> This will trigger all bots that don't restrict targets to build VE,
> and you may get some failures here already.
I see no harm in moving clang-ve-ninja to main buildbot while we are
watching the ramifications play out for all builders. Builders don't
interfere with one another. There will only be better coverage.

That's not what I mean. I also see no harm in moving the VE bot before the move, as it's meant to be passing all the time.

What I meant is that the move to production (D113247) could break other bots that will build VE if they don't have an exclusive list of targets to build.

Not because of the VE backend, we know it's fine (due to VE's own buildbot). But because the other bots may have a build option/environment that you haven't tested and will need to be sorted quickly.

Most likely nothing will happen, but I'm just making sure we expect a potential problem, and perhaps give it a day's worth of waiting after moving to production to continue.

This only matters for the clang-ve-ninja builder. The llvm-zorg patch
moves 'VE' from the experimental target to the targets_to_build cmake
option for clang-ve-ninja and enables reporting on failure. This will
not effect other bots.

Ack.

[Simon Moll via llvm-dev]

Yep. There definitely could be issues on untested build environments.
Waiting for that to play out and patching our way back to green
pastures is the final, 4th step in the plan. Steps 1-3 are all about
adding VE to the official target list and migrating the bot. That's
all. VE is on a feature freeze until all bots appear stable.

>
> > This only matters for the clang-ve-ninja builder. The llvm-zorg
> > patch
> > moves 'VE' from the experimental target to the targets_to_build
> > cmake
> > option for clang-ve-ninja and enables reporting on failure. This
> > will
> > not effect other bots.
> >
>
>
> Ack.

[Renato Golin via llvm-dev]

On Tue, 30 Nov 2021 at 08:36, Simon Moll <Simon...@emea.nec.com> wrote:

Yep. There definitely could be issues on untested build environments.
Waiting for that to play out and patching our way back to green
pastures is the final, 4th step in the plan. Steps 1-3 are all about
adding VE to the official target list and migrating the bot. That's
all. VE is on a feature freeze until all bots appear stable.

I see I misread your first proposal. LGTM, thanks! 

[Simon Moll via llvm-dev]

No worries :) We are now waiting for acceptance of D113247 to start the
process.

[Simon Moll via llvm-dev]

FYI The VE target is now in the official target list. We are following
up on the bots to react to any build failures.

I keep a protocol in the Diff as it plays out:
https://reviews.llvm.org/D113247