llvm dev - Nov 2021 - Proposal: Make the VE target official

On Wed, 3 Nov 2021 at 12:52, Simon Moll <Simon.Moll at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211108/ea499eea/attachment.html>

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.Moll at 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