[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE ${LLVM_LIBRARY_DIR}/TargetParser/)
----------------
fpetrogalli wrote:
> lenary wrote:
> > fpetrogalli wrote:
> > > craig.topper wrote:
> > > > fpetrogalli wrote:
> > > > > lenary wrote:
> > > > > > craig.topper wrote:
> > > > > > > Why do we need to touch CMake file that aren't RISC-V?
> > > > > > Yeah, this shouldn't be needed.
> > > > > > 
> > > > > > We do have some fixes for the modules build which recently landed, maybe they fix the issues you were seeing, including:
> > > > > > - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > > > > - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > > > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might clear up some weird things about the quicker modulemap fixes)
> > > > > This is unrelated to Modules. The .inc file generated by tablegen is created in `{make_build_folder}/lib/TargetParser`. The file is then included in `TargetParser.cpp` but also in `TargetParser.h` -> this means that every time we include the latter in a cpp file we need to make the inc file visible for inclusion.
> > > > Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
> > > Of course! :) I'll do it in a separate patch.
> > I think, if that is the case, we should add `{make_build_folder}/lib/TargetParser` as a public include directory of the LLVMTargetParser library, which should mean anything depending on it gets that as an include directory they can rely on.
> > I think, if that is the case, we should add `{make_build_folder}/lib/TargetParser` as a public include directory of the LLVMTargetParser library, which should mean anything depending on it gets that as an include directory they can rely on.
> 
> How do I do that?I tried `target_include_directories(LLVMTargetParser PUBLIC ${LLVM_LIBRARY_DIR}/TargetParser/)` but I got: 
> ```
> CMake Error in lib/TargetParser/CMakeLists.txt:
>   Target "LLVMTargetParser" INTERFACE_INCLUDE_DIRECTORIES property contains
>   path:
> 
>     "/Users/fpetrogalli/<...>/build-shared/./lib/TargetParser/"
> 
>   which is prefixed in the build directory.
> 
> ```
I think https://stackoverflow.com/a/25681179 points to how to do this in such a way that it works properly for people using cmake install. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137517/new/

https://reviews.llvm.org/D137517