llvm dev - Feb 2020 - Whose responsibility is it to maintain tests using experimental backends?

Hi all,

I recently, on behalf of a new LLVM contributor, pushed commit  740bc366d44c
<https://reviews.llvm.org/rG740bc366d44ccd41161739bc1d4b447cd49aba65>,
which broke an in-tree test for the AVR backend (see
http://45.33.8.238/mac/7865/step_11.txt). On the review
https://reviews.llvm.org/D72992, a request was made to fix the test or
revert the change. This test didn't fail locally or on any of the build
bots, because the AVR backend is experimental and does not run even for
build that has all normal targets enabled).

The fix in this case is simple, and I don't have any real issue in making
it, but there's a wider principle here: if an in-tree test fails but only
when experimental items are enabled, whose responsibility is it to fix the
issue? I'm inclined to think that it's the responsibility of whoever
maintains the experimental target and/or the bot maintainer where the
experimental target is enabled, and NOT the regular developer.

Thoughts?

James
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200212/d8722e6c/attachment.html>

> On Feb 12, 2020, at 08:45, James Henderson via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi all,
> 
> I recently, on behalf of a new LLVM contributor, pushed commit 
740bc366d44c
<https://reviews.llvm.org/rG740bc366d44ccd41161739bc1d4b447cd49aba65>,
which broke an in-tree test for the AVR backend (see
http://45.33.8.238/mac/7865/step_11.txt
<http://45.33.8.238/mac/7865/step_11.txt>). On the review
https://reviews.llvm.org/D72992 <https://reviews.llvm.org/D72992>, a
request was made to fix the test or revert the change. This test didn't fail
locally or on any of the build bots, because the AVR backend is experimental and
does not run even for build that has all normal targets enabled).
> 
> The fix in this case is simple, and I don't have any real issue in
making it, but there's a wider principle here: if an in-tree test fails but
only when experimental items are enabled, whose responsibility is it to fix the
issue? I'm inclined to think that it's the responsibility of whoever
maintains the experimental target and/or the bot maintainer where the
experimental target is enabled, and NOT the regular developer.
> 
> Thoughts?
> 
> 
This is why I have never liked the experimental target approach. Having
experimental targets only increases the burden for other developers. Many times
I’ve changed some target API, and broken the build for the experimental targets.
It creates strictly more work for me, since now I have to see the build bot
failed, and then go back to do a new build with the targets enabled to fix it.
It would be easier to just always build all targets by default.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200212/68381caf/attachment.html>

https://llvm.org/docs/DeveloperPolicy.html#new-targets says that
experimental backends should be supported by an active community, in part
by providing build bots and fixing bugs related to the backend. So while it
doesn't *explicitly* say things about patches that only break those
backends, I think reading the spirit of that policy says that it's the
responsibility of the experimental backend's owners to fix the issue. You
can still be nice and fix the issue yourself, but if you can't, that
isn't
grounds for anyone to revert the patch.

On Wed, Feb 12, 2020 at 6:48 AM Matt Arsenault via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
>
> On Feb 12, 2020, at 08:45, James Henderson via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hi all,
>
> I recently, on behalf of a new LLVM contributor, pushed commit
> 740bc366d44c
>
<https://reviews.llvm.org/rG740bc366d44ccd41161739bc1d4b447cd49aba65>,
> which broke an in-tree test for the AVR backend (see
> http://45.33.8.238/mac/7865/step_11.txt). On the review
> https://reviews.llvm.org/D72992, a request was made to fix the test or
> revert the change. This test didn't fail locally or on any of the build
> bots, because the AVR backend is experimental and does not run even for
> build that has all normal targets enabled).
>
> The fix in this case is simple, and I don't have any real issue in
making
> it, but there's a wider principle here: if an in-tree test fails but
only
> when experimental items are enabled, whose responsibility is it to fix the
> issue? I'm inclined to think that it's the responsibility of
whoever
> maintains the experimental target and/or the bot maintainer where the
> experimental target is enabled, and NOT the regular developer.
>
> Thoughts?
>
>
>
> This is why I have never liked the experimental target approach. Having
> experimental targets only increases the burden for other developers. Many
> times I’ve changed some target API, and broken the build for the
> experimental targets. It creates strictly more work for me, since now I
> have to see the build bot failed, and then go back to do a new build with
> the targets enabled to fix it. It would be easier to just always build all
> targets by default.
>
> -Matt
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200212/13faf0ef/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200212/13faf0ef/attachment.bin>