Page MenuHomePhabricator

[doc] Clarify responsibility for fixing experimental target problems
ClosedPublic

Authored by jhenderson on Feb 13 2020, 2:32 AM.

Details

Summary

Experimental targets are meant to be maintained by the community behind the target. They are not monitored by the primary build bots. This change clarifies that it is this communities responsibility for things like test fixes related to the target caused by changes unrelated to that target.

See http://lists.llvm.org/pipermail/llvm-dev/2020-February/139115.html for a full discussion.

Diff Detail

Event Timeline

jhenderson created this revision.Feb 13 2020, 2:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2020, 2:32 AM

Upload correct diff with better wording.

Fine with me, but I think the more interesting conversation to have is one about the future of the AVR target. The AVR target is the only experimental target that currently exists.

It's been in the tree for a while now, so it should either become stable or move out of tree again.

The last commit by the code owner is from July 2019. I don't have any relation with the AVR target except that I have a bot that builds with all targets enabled. Nobody seems to be fixing AVR issues, and git log llvm/lib/Target/AVR only shows global refactoring changes modifying that directory.

So this kind of suggests "remove the AVR target".

On the other hand, e.g. XCore is in even worse shape (code owner inactive since Aug 2014) and that's a non-experimental target. And it's not like AVR needs all that much maintenance. So we could make it non-experimental as well.

I don't have an opinion on either. But it seems like experimental targets shouldn't stay experimental for many years.

https://reviews.llvm.org/p/dylanmckay/ looks pretty active. So I think promoting AVR to non-experimental might make sense.

rupprecht accepted this revision.Feb 13 2020, 10:16 AM

Thanks James!

Fine with me, but I think the more interesting conversation to have is one about the future of the AVR target. The AVR target is the only experimental target that currently exists.

It's been in the tree for a while now, so it should either become stable or move out of tree again.

The last commit by the code owner is from July 2019. I don't have any relation with the AVR target except that I have a bot that builds with all targets enabled. Nobody seems to be fixing AVR issues, and git log llvm/lib/Target/AVR only shows global refactoring changes modifying that directory.

So this kind of suggests "remove the AVR target".

On the other hand, e.g. XCore is in even worse shape (code owner inactive since Aug 2014) and that's a non-experimental target. And it's not like AVR needs all that much maintenance. So we could make it non-experimental as well.

I don't have an opinion on either. But it seems like experimental targets shouldn't stay experimental for many years.

Yes, I don't have an opinion either way, but just to be clear, this is an orthogonal discussion that's unrelated to this patch. I'd recommend starting a thread on llvm-dev.

This revision is now accepted and ready to land.Feb 13 2020, 10:16 AM
MaskRay accepted this revision.Feb 13 2020, 11:47 AM
MaskRay added a subscriber: MaskRay.
MaskRay added inline comments.
llvm/docs/DeveloperPolicy.rst
576

This cmake variable is LLVM_EXPERIMENTAL_TARGETS_TO_BUILD

For example, to enable all currently experimental targets:

-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD='ARC;AVR;VE'

lattner accepted this revision.Feb 13 2020, 3:40 PM

Very nice, please correct the CMAKE variable or, better yet, just drop the explicit mention of the flag to pass). I really appreciate that you are improving our policies!

Removed reference to CMake variable.

jhenderson marked an inline comment as done.Feb 14 2020, 1:48 AM

Thanks for the comments! I've updated the text to remove reference to the explicit variable, and will go ahead and push this shortly.

Regarding the future of the AVR and other experimental targets, that definitely should be brought up on the mailing list in a separate discussion. This change should just be codifying what the already generally understood policy is.

This revision was automatically updated to reflect the committed changes.