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

Reply via email to