[RISCV] RISCVTargetParserDef.inc doesn't get rebuilt when RISCVProcessors.td is modified

[tclin914]

There are a couple of post-commit CI failures (#144022) after we made changes to the CPU definitions.

The reason is that RISCVTargetParserDef.inc does not get rebuilt when RISCVProcessors.td is modified, because the build rule for RISCVTargetParserDef.inc does not declare a dependency on RISCVProcessors.td, as shown in build.ninja.
See

llvm-project/llvm/cmake/modules/TableGen.cmake

Line 47 in ad9e591

else()

for more details.

The CMake version should be less than 3.23; otherwise, it will use depfiles (.inc.d) instead.

Should we explicitly add all the .td files under llvm/lib/Target/RISCV/ as dependencies of RISCVTargetParserDef.inc , similar to file(GLOB_RECURSE global_tds "${LLVM_MAIN_INCLUDE_DIR}/llvm/*.td")?

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Jim Lin (tclin914)

There are a couple of post-commit CI failures (https://github.com//pull/144022) after we made changes to the CPU definitions.

The reason is that RISCVTargetParserDef.inc does not get rebuilt when RISCVProcessors.td is modified, because the build rule for RISCVTargetParserDef.inc does not declare a dependency on RISCVProcessors.td, as shown in build.ninja.
See

llvm-project/llvm/cmake/modules/TableGen.cmake

Line 47 in ad9e591

else()

for more details.

The CMake version should be less than 3.23; otherwise, it will use depfiles (.inc.d) instead.

Should we explicitly add all the .td files under llvm/lib/Target/RISCV/ as dependencies of RISCVTargetParserDef.inc , similar to file(GLOB_RECURSE global_tds "${LLVM_MAIN_INCLUDE_DIR}/llvm/*.td")?

[lenary]

Good spot.

What does the .inc.d file contain? Presumably it's not there in a clean builds, but with an existing build for it should list all the included tablegen files, including the missing one? So I'd expect the issue to be with clean builds only?

If you're saying before cmake 2.23 we should be adding this to the global list (because depfiles don't work), surely the file should already be in the local files list? Or is something going wrong with that.

[tclin914]

For CMake version 3.23 and above, the .inc.d file (in this case, RISCVTargetParserDef.inc.d) includes a list of all *.td files under llvm/lib/Target/RISCV/(since they are included in RISCV.td) and treats them as dependencies during the build process.
As a result, any change made to those .td files will trigger a rebuild of RISCVTargetParserDef.inc.

For CMake versions below 3.23, RISCVTargetParserDef.inc only depends on RISCV.td and does not include the .td files that RISCV.td itself includes.
That’s why changes to RISCVProcessors.td do not trigger a rebuild of RISCVTargetParserDef.inc.

One key point is that RISCVTargetParserDef.inc is located in include/llvm/TargetParser/, so the *.td files under lib/Target/RISCV/ are not considered part of the local file list.

[tclin914]

So I'd expect the issue to be with clean builds only?

I believe this issue does not occur with clean builds. In https://lab.llvm.org/buildbot/#/builders/42/builds/4981, the clean stage does nothing.

[lenary]

One key point is that RISCVTargetParserDef.inc is located in include/llvm/TargetParser/, so the *.td files under lib/Target/RISCV/ are not considered part of the local file list.

Ah, right, this is the problem then? Can we maybe fix how local files are globbed?