[PATCH] D109260: [RISCV] Add SiFive cores E and S series

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/test/Misc/target-invalid-cpu-note.c:195
 // RISCV32: error: unknown target CPU 'not-a-cpu'
-// RISCV32: note: valid target CPU values are: generic-rv32, rocket-rv32, sifive-7-rv32, sifive-e31, sifive-e76
+// RISCV32: note: valid target CPU values are: generic-rv32, rocket-rv32, sifive-7-rv32, sifive-e20,  sifive-e21,  sifive-e24, sifive-e31, sifive-e34, sifive-e76
 
----------------
Why 2 spaces after commas here?


================
Comment at: llvm/lib/Target/RISCV/RISCV.td:279
+
+def : ProcessorModel<"sifive-s21", RocketModel, [Feature64Bit,
+                                                 FeatureStdExtM,
----------------
apivovarov wrote:
> craig.topper wrote:
> > Can we sort these by leading digit, then by letter? That will keep most of the SiFive7Models together.
> Well, it is sorted by `core_type + number` in all other places in LLVM codebase (including error messages). GCC also sorts them as [[ https://github.com/gcc-mirror/gcc/blob/master/gcc/config/riscv/riscv-cores.def | E-S-U ]]. As well as [[ https://www.sifive.com/risc-v-core-ip | sifive.com ]]
Ok. I was basing that request on our internal codebase where we have them sorted the other way. But it doesn't matter a lot to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109260