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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/TargetParser/CMakeLists.txt:3
+tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def -I ${CMAKE_SOURCE_DIR}/lib/Target/RISCV/)
+add_public_tablegen_target(RISCVTargetParserTableGen)
+
----------------
fpetrogalli wrote:
> thakis wrote:
> > All other target tablegen'ing for all other targets (also for RISCV for other .td changes) happen in llvm/lib/Target, not in llvm/TargetParser. Is there any way this could be structured that way too? As-is, the layering looks pretty unusual. (And your reland had to teach clang about tablegen targets for that reason.)
> Thanks for the feedback.
> 
> I can think of two possible solutions:
> 
> 1. Move the RISCV-only part of `llvm/lib/TargetParser` into `llvm/lib/Target`
> 2. Move the whole TargetParser inside Target
> 
> However, I am not really sure if these are less unusual than the current state of things (or even technically possible).
> 
>  I am open for suggestions, and even more open to review a solution that fixes the unusual layering ;).
> 
I could be wrong, but I don't think clang depends on the LLVM's Target libraries today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517