[clang] [clang] Only build clang-tblgen if it is actually needed (PR #161952)

[David Spickett via cfe-commits]

DavidSpickett wrote:

For this cmake command:
```
$ cmake ../llvm-project/llvm/ -DCMAKE_BUILD_TYPE=Release -DCLANG_TABLEGEN_EXE=/home/david.spickett/build-llvm-aarch64/bin/clang-tblgen -DLLVM_CCACHE_BUILD=ON -G Ninja -DLLVM_ENABLE_PROJECTS="clang;llvm" -DLLVM_INCLUDE_TESTS=ON
```
This patch works for me:
```
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index e4cb1a359620..b650b3b986f4 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -479,7 +479,9 @@ option(CLANG_ENABLE_HLSL "Include HLSL build products" Off)
 # While HLSL support is experimental this should stay hidden.
 mark_as_advanced(CLANG_ENABLE_HLSL)
 
-add_subdirectory(utils/TableGen)
+if (NOT DEFINED CLANG_TABLEGEN_EXE OR CLANG_INCLUDE_TESTS)
+  add_subdirectory(utils/TableGen)
+endif()
 
 # Export CLANG_TABLEGEN_EXE for use by flang docs.
 set(CLANG_TABLEGEN_EXE "${CLANG_TABLEGEN_EXE}" CACHE INTERNAL "")
```
Even though it seems to do the same thing you did, but one level up. I wonder if it's a variable scope difference.

Regardless, we'd usually do this at the point where we'd call add_subdirectory, rather than in the subdirectory itself.

Though we do not say either way the meaning of providing and existing clang-tblgen and running tests, it does currently run tests that use the newly built clang-tblgen. It's not unreasonable to think someone would want to test the clang they'd just built, and expecting them to expect that this requires some random DSL compiler, that's a bit much.

In theory someone could be relying on there being another clang-tblgen in bin, maybe for installs, but that seems like a thing we can afford to break. They were building clang-tblgen anyway, so they could stop setting CLANG_TABLEGEN_EXE, and use the newly built one instead.

Maybe they are building debug but giving a path to release but for that, we have a specific option to not apply debug settings to tablegen.

So I think the way to think about it is, we need to build a new clang-tblgen when:
* CLANG_TABLEGEN_EXE is not provided, or -
* CLANG_INCLUDE_TESTS is ON

If you want tests on and no extra copy of clang-tblgen then that's going to require adding a new testing feature for the few clang tests that require clang-tblgen. Which isn't that difficult but I don't think we have to go that far given that most people will be disabling tests altogether if they care about build time.

Making it so that clang tests using clang-tblgen use the one from CLANG_TABLEGEN_EXE is confusing and I don't think the results would be meaningful. The in-tree tests are written expecting the newest tablegen, so it's likely the behaviour will get out of sync. So definitely don't do that.

https://github.com/llvm/llvm-project/pull/161952