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

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added inline comments.


================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:15
+#include "llvm/TableGen/Record.h"
+#include <iostream>
+namespace llvm {
----------------
barannikov88 wrote:
> <iostream> [[ https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden | is forbidden ]] by the coding standards.
> 
Ops - that was a leftover for my nasty debugging session :)


================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:16
+#include <iostream>
+namespace llvm {
+
----------------
barannikov88 wrote:
> This [[ https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions | should be ]] `using namespace llvm;`
Hum, if I do this, I get:

```
Undefined symbols for architecture arm64:
  "llvm::EmitRISCVTargetDef(llvm::RecordKeeper&, llvm::raw_ostream&)", referenced from:
      (anonymous namespace)::LLVMTableGenMain(llvm::raw_ostream&, llvm::RecordKeeper&) in TableGen.cpp.o
ld: symbol(s) not found for architecture arm64
```

It is a bit surprising because the linking command has `utils/TableGen/CMakeFiles/llvm-tblgen.dir/RISCVTargetDefEmitter.cpp.o` into it... Some of the files in this folder do not use the convention you pointed at, it it OK if I live it as it is?


================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:28
+
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
----------------
barannikov88 wrote:
> This function does not seem to mutate RecordKeeper, so it should be `const`.
Done, however none of the other "emitters" in `TableGenBackends.h` mark the record keeper as const...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517