Skip to content

[TargetParser][cmake] Be Smarter about TableGen Deps #144848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 20, 2025

Conversation

lenary
Copy link
Member

@lenary lenary commented Jun 19, 2025

This tries to be a bit smarter for the OLD behaviour of CMP0116, to glob more relevant directories looking for possible dependencies.

The changes are:

  • Remove some duplication of lines in the tablegen function.
  • Put CURRENT_SOURCE_DIR into tblgen_includes (at the front)
  • Glob all directories in tblgen_includes
  • Give up on local_tds which was wrong when using tablegen to compile a file in a different directory (as TargetParser does)
  • Use EXTRA_INCLUDES in TargetParser tablegen calls.

This is still an under-approximation of what might be included, at least comparing the RISCVTargetParserDef.inc.d (after building target_parser_gen), and the list of deps in the ninja file when explicitly setting CMP0116 to OLD.

Fixes #144639

This tries to be a bit smarter for the OLD behaviour of CMP0116, to glob
more relevant directories looking for possible dependencies.

The changes are:
- Remove some duplication of lines in the `tablegen` function.
- Put CURRENT_SOURCE_DIR into `tblgen_includes` (at the front)
- Glob all directories in `tblgen_includes`
- Give up on `local_tds` which was wrong when using tablegen to compile
  a file in a different directory (as TargetParser does)
- Use `EXTRA_INCLUDES` in TargetParser `tablegen` calls.

This is still an under-approximation of what might be included, at least
comparing the RISCVTargetParserDef.inc.d (after building
`target_parser_gen`), and the list of deps in the ninja file when
explicitly setting CMP0116 to OLD.

Fixes llvm#144639
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Jun 19, 2025
Copy link
Contributor

@tclin914 tclin914 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed every line of this change — it looks good to me. I also tested it locally, and it works well. I hope I didn't miss anything.

@lenary lenary merged commit ab8b8c1 into llvm:main Jun 20, 2025
7 checks passed
@lenary lenary deleted the pr/targetparser-tablegen-deps branch June 20, 2025 18:05
tclin914 added a commit that referenced this pull request Jun 22, 2025
…cpus." (#144402)"

Since the fix #144848 for post-commit CI failure
has landed.

This reverts commit f83d09a.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 22, 2025
…des series cpus." (#144402)"

Since the fix llvm/llvm-project#144848 for post-commit CI failure
has landed.

This reverts commit f83d09a.
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
…cpus." (llvm#144402)"

Since the fix llvm#144848 for post-commit CI failure
has landed.

This reverts commit f83d09a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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