fpetrogalli marked an inline comment as done. fpetrogalli 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) + ---------------- craig.topper wrote: > 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. @thakis - is D141581 a less unusual solution? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/ https://reviews.llvm.org/D137517 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits