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

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

In D137517#4009328 <https://reviews.llvm.org/D137517#4009328>, @jrtc27 wrote:

> Hm, this means that llvm/lib/Target/$TARGET/$TARGET.td needs to remain working (or at least mostly working, things like ISel patterns can fail to type check) in order for Clang to build, since we're now introducing a dependency on llvm/lib/Target/$TARGET in Clang where there didn't used to be (regardless of whether you care about that target). That's probably fine but worth pointing out, especially in the context of experimental targets that are supposed to be mostly ignorable.

Yes,`llvm/lib/Target/$TARGET/$TARGET.td` need to remain working. But this is true only for those targets that have an auto generative process of the list of CPUs and features needed by the target parser. Right now the only target having this is RISCV. Any target do not need to worry about the dependencies introduced by this patch , until they introduce their own custom generator.



================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:16
+#include <iostream>
+namespace llvm {
+
----------------
barannikov88 wrote:
> fpetrogalli wrote:
> > 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?
> Right, after `using namespace llvm` you have to write
> `llvm::EmitRISCVTargetDef` with explicit `llvm::` qualification. This is the whole point of this guideline :)
> Please see the [[ https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions | link ]].
> 
Yeah, I did that, I just missed adding the declaration via header file inclusion...

```
@@ -12,8 +12,8 @@
 //===----------------------------------------------------------------------===//

 #include "llvm/TableGen/Record.h"
-
-namespace llvm {
+#include "TableGenBackends.h"
+using namespace llvm;

 static std::string getEnumFeatures(Record &Rec) {
   std::vector<Record *> Features = Rec.getValueAsListOfDefs("Features");
@@ -25,7 +25,7 @@ static std::string getEnumFeatures(Record &Rec) {
   return "FK_NONE";
 }

-void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517