Make LLVM_TARGETS_TO_BUILD=all build all targets

[rsmith]

Summary

For LLVM development, we want a mode that builds everything; this is what the buildbots that say "all targets" should be testing. (All code in tree should build and pass its tests, even if it's an experimental target.) However, LLVM_TARGETS_TO_BUILD=all currently (surprisingly) only builds non-experimental targets. (Additional targets can be specified via LLVM_EXPERIMENTAL_TARGETS_TO_BUILD, which confusingly is just a list of more targets to build in addition to those in LLVM_TARGETS_TO_BUILD.)

This patch addresses this as follows:

• LLVM_TARGETS_TO_BUILD=all builds all targets
• A new value, LLVM_TARGETS_TO_BUILD=stable, builds only non-experimental targets
• The default for LLVM_TARGETS_TO_BUILD is changed from all to stable.

I've left LLVM_EXPERIMENTAL_TARGETS_TO_BUILD alone for now. I think it'd make more sense to be able to specify something like LLVM_TARGETS_TO_BUILD=stable;WebAssembly to add experimental targets to the stable targets list, but I don't see a reason to couple such a change to this change.

[MatzeB]

Changing the meaning of "all" to really mean all targets makes sense. But there is of course a fundamental discussion here:

I think we explicitely do NOT build the experimental targets in any of the normal buildbots. I think it is fair to leave the burden of updating the experimental targets to whoever maintains them. I do make sure that I do not break compilation of experimental targets when I make changes to the API. However I do not want to start debugging and updating even more tests. This is always a huge pita on architectures you don't know, have no simulators/documentation about, may just have bugs in the target specific code that are triggered by your changes etc.
To have an example: Let's say a valid CodeGen change produces slightly different code that hits a "Not Implemented Yet" assert in the experimental target, I don't wanna implement that just to keep the tests running. Even XFAILing the affected tests, filing PRs for them etc. is a big hassle.

We should leave that reponsibility with the maintainers of the experimental targets. Which means we should only have "silent" bots for the experimental targets, i.e. only the target maintainers get failure mails and they are responsible for fixing their target.

[silvas]

IIRC the intention was that in-tree work should not be burdened by experimental targets, up to and including breaking them. E.g. an in-tree API migration wouldn't have to update experimental targets. So in that sense LLVM_TARGETS_TO_BUILD=all is working as intended I think.

Maybe Renato remembers more details?

[mehdi_amini]

In D34910#797402, @silvas wrote:

IIRC the intention was that in-tree work should not be burdened by experimental targets, up to and including breaking them. E.g. an in-tree API migration wouldn't have to update experimental targets. So in that sense LLVM_TARGETS_TO_BUILD=all is working as intended I think.

Yes this is correct: but what matters on this aspect is the *default* isn't it? A bot does not have to specify LLVM_TARGETS_TO_BUILD, and as long as the default does not include the experimental target that seems fine to me.

[MatzeB]

In D34910#797469, @mehdi_amini wrote:

In D34910#797402, @silvas wrote:

IIRC the intention was that in-tree work should not be burdened by experimental targets, up to and including breaking them. E.g. an in-tree API migration wouldn't have to update experimental targets. So in that sense LLVM_TARGETS_TO_BUILD=all is working as intended I think.

Yes this is correct: but what matters on this aspect is the *default* isn't it? A bot does not have to specify LLVM_TARGETS_TO_BUILD, and as long as the default does not include the experimental target that seems fine to me.

Yes the patch as is is perfectly fine and we can push it right away.
However it obviously smells like the next thing happening is a commit to zorg putting LLVM_TARGET_TO_BUILD=all on some buildbots. Before this patch it wasn't possible to do that so conveniently as this. So I am raising my complaint early so nobody does that hypethical zorg commit without discussion on llvm-dev!

[silvas]

Another possible naming convention that might avoid the scenario Matthias is worried about is all/everything like clang's -W options.

[MatzeB]

In D34910#797530, @silvas wrote:

Another possible naming convention that might avoid the scenario Matthias is worried about is all/everything like clang's -W options.

I actually find -Weverything vs. -Wall rather silly (but understandable from history). For LLVM_TARGETS_TO_BUILD I'd rather just have all.

[chapuni]

I prefer "everything", but "stable/all" would be better since they are more descriptive.

[rengolin]

Changing the default to stable removes any concerns about zorg. I quite like this change as I do agree that all should be all. I also don't like the -W distinction, and this is where this change makes sense.

If we can still use the old way to build experimental targets, this LGTM. If not, as in we build all or nothing, then maybe the proposed change to have "stable;Foo" would be nice to have if not now, very soon.

CMakeLists.txt

 set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples)
 set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
 
-set(LLVM_ALL_TARGETS
+set(LLVM_STABLE_TARGETS
   AArch64
   AMDGPU
   ARM

   XCore
   )
 
+set(LLVM_ALL_TARGETS
+  ${LLVM_STABLE_TARGETS}
+  AVR
+  Nios2
+  WebAssembly
+  )
+
 # List of targets with JIT support:
 set(LLVM_TARGETS_WITH_JIT X86 PowerPC AArch64 ARM Mips SystemZ)
 
-set(LLVM_TARGETS_TO_BUILD "all"
-    CACHE STRING "Semicolon-separated list of targets to build, or \"all\".")
+set(LLVM_TARGETS_TO_BUILD "stable"
+    CACHE STRING "Semicolon-separated list of targets to build, or \"stable\", or \"all\".")
 
 set(LLVM_EXPERIMENTAL_TARGETS_TO_BUILD ""
   CACHE STRING "Semicolon-separated list of experimental targets to build.")

 
 option(LLVM_ENABLE_ZLIB "Use zlib for compression/decompression if available." ON)
 
-if( LLVM_TARGETS_TO_BUILD STREQUAL "all" )
+if( LLVM_TARGETS_TO_BUILD STREQUAL "stable" )
+  set( LLVM_TARGETS_TO_BUILD ${LLVM_STABLE_TARGETS} )
+elseif( LLVM_TARGETS_TO_BUILD STREQUAL "all" )
   set( LLVM_TARGETS_TO_BUILD ${LLVM_ALL_TARGETS} )
 endif()