[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/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE ${LLVM_LIBRARY_DIR}/TargetParser/)
----------------
Why do we need to touch CMake file that aren't RISC-V?


================
Comment at: llvm/lib/Target/RISCV/RISCV.td:580
+def SIFIVE_7 : RISCVProcessorModelTUNE_PROC<"sifive-7-series", SiFive7Model, [],
                      [TuneSiFive7]>;
 
----------------
Line this up to the column after the `<` on the previous line.


================
Comment at: llvm/lib/Target/RISCV/RISCV.td:587
+def SIFIVE_E21 : RISCVProcessorModelPROC<"sifive-e21", RocketModel, [Feature32Bit,
                                                  FeatureStdExtM,
                                                  FeatureStdExtA,
----------------
Line this up under `Feature32Bit` on the previous line.


================
Comment at: llvm/utils/TableGen/CMakeLists.txt:63
   CTagsEmitter.cpp
+  RISCVTargetDefEmitter.cpp
   )
----------------
I think this list might be in alphabetical order except for the placement of CTagsEmitter. Can you move RISCVTargetDefEmitter.cpp into the right place?


================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:10
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISCV CPUs.
+//
----------------
RISCV->RISC-V


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517