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

[Francesco Petrogalli via Phabricator via cfe-commits]

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

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

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added a comment.

FYI, the CMake file should use `PROJECT_SOURCE_DIR` instead of 
`CMAKE_SOURCE_DIR`, otherwise it breaks builds that use CMake’s 
add_subdirectory. I put up D141521  to fix 
that.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D137517#4045337 , @jrtc27 wrote:

> In D137517#4045315 , @fpetrogalli 
> wrote:
>
>> In D137517#4045299 , @jrtc27 wrote:
>>
>>> In D137517#4042875 , @fpetrogalli 
>>> wrote:
>>>
 In D137517#4042758 , 
 @fpetrogalli wrote:

> After submitting this, I had to revert it 
> 
>  because of failures like 
> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio

 I have resubmitted with what I hope is the right fix (I could not 
 reproduce any of the failures I was seeing in buildbot, on my machine the 
 build is fine).

 The new commit is at 
 https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
  - in the commit message I have explained what I have changed WRT this 
 original patch. I have added  the
 tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
 `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
 theist o the CPU is available even if `LLVMTargetParser` has not been 
 built yet.
>>>
>>> But you didn't use the proper Differential Revision tag, so the diff here 
>>> didn't get updated to reflect the amended version committed :(
>>
>> What should I have done? Add  the `Differential Revision: 
>> https://reviews.llvm.org/D137517` as the last line of the commit with the 
>> rework? I wasn't aware of this for reworks.
>
> Yes, it should have the same trailer as the original commit, otherwise it 
> won't be correctly tracked by Phabricator. A "rework" isn't special, it's 
> revert, reopen the revision, update the revision and land the revision again. 
> If re-review isn't needed then you can skip some of the middle, but that's 
> it. Though in this case I do think re-review was warranted, the new clang 
> dependency seems a bit dubious and hints at the design not being quite right.

My bad -  I assumed that adding such tablegenning dependency in clang was a 
minor thing. I'll see if I can remove it. Any suggestions on how this could be 
done without duplicating the information in RISCV.td?


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

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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D137517#4045315 , @fpetrogalli 
wrote:

> In D137517#4045299 , @jrtc27 wrote:
>
>> In D137517#4042875 , @fpetrogalli 
>> wrote:
>>
>>> In D137517#4042758 , @fpetrogalli 
>>> wrote:
>>>
 After submitting this, I had to revert it 
 
  because of failures like 
 https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio
>>>
>>> I have resubmitted with what I hope is the right fix (I could not reproduce 
>>> any of the failures I was seeing in buildbot, on my machine the build is 
>>> fine).
>>>
>>> The new commit is at 
>>> https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
>>>  - in the commit message I have explained what I have changed WRT this 
>>> original patch. I have added  the
>>> tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
>>> `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
>>> theist o the CPU is available even if `LLVMTargetParser` has not been built 
>>> yet.
>>
>> But you didn't use the proper Differential Revision tag, so the diff here 
>> didn't get updated to reflect the amended version committed :(
>
> What should I have done? Add  the `Differential Revision: 
> https://reviews.llvm.org/D137517` as the last line of the commit with the 
> rework? I wasn't aware of this for reworks.

Yes, it should have the same trailer as the original commit, otherwise it won't 
be correctly tracked by Phabricator. A "rework" isn't special, it's revert, 
reopen the revision, update the revision and land the revision again. If 
re-review isn't needed then you can skip some of the middle, but that's it. 
Though in this case I do think re-review was warranted, the new clang 
dependency seems a bit dubious and hints at the design not being quite right.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added a comment.

In D137517#4045299 , @jrtc27 wrote:

> In D137517#4042875 , @fpetrogalli 
> wrote:
>
>> In D137517#4042758 , @fpetrogalli 
>> wrote:
>>
>>> After submitting this, I had to revert it 
>>> 
>>>  because of failures like 
>>> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio
>>
>> I have resubmitted with what I hope is the right fix (I could not reproduce 
>> any of the failures I was seeing in buildbot, on my machine the build is 
>> fine).
>>
>> The new commit is at 
>> https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
>>  - in the commit message I have explained what I have changed WRT this 
>> original patch. I have added  the
>> tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
>> `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
>> theist o the CPU is available even if `LLVMTargetParser` has not been built 
>> yet.
>
> But you didn't use the proper Differential Revision tag, so the diff here 
> didn't get updated to reflect the amended version committed :(

What should I have done? Add  the `Differential Revision: 
https://reviews.llvm.org/D137517` as the last line of the commit with the 
rework? I wasn't aware of this for reworks.


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

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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D137517#4042875 , @fpetrogalli 
wrote:

> In D137517#4042758 , @fpetrogalli 
> wrote:
>
>> After submitting this, I had to revert it 
>> 
>>  because of failures like 
>> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio
>
> I have resubmitted with what I hope is the right fix (I could not reproduce 
> any of the failures I was seeing in buildbot, on my machine the build is 
> fine).
>
> The new commit is at 
> https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
>  - in the commit message I have explained what I have changed WRT this 
> original patch. I have added  the
> tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
> `clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
> theist o the CPU is available even if `LLVMTargetParser` has not been built 
> yet.

But you didn't use the proper Differential Revision tag, so the diff here 
didn't get updated to reflect the amended version committed :(


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

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

[Craig Topper via Phabricator via cfe-commits]

craig.topper 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)
+

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.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

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)
+

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 ;).



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

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

[Nico Weber via Phabricator via cfe-commits]

thakis 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)
+

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.)


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D137517#4042758 , @fpetrogalli 
wrote:

> After submitting this, I had to revert it 
> 
>  because of failures like 
> https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio

I have resubmitted with what I hope is the right fix (I could not reproduce any 
of the failures I was seeing in buildbot, on my machine the build is fine).

The new commit is at 
https://github.com/llvm/llvm-project/commit/ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf
 - in the commit message I have explained what I have changed WRT this original 
patch. I have added  the
tablegen target `RISCVTargetParserTableGen` in the `DEPENDS` list of 
`clangDriver` and `clangBasic`. This makes sure that the `.*inc` file with 
theist o the CPU is available even if `LLVMTargetParser` has not been built yet.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

After submitting this, I had to revert it 

 because of failures like 
https://lab.llvm.org/buildbot/#/builders/225/builds/12367/steps/5/logs/stdio


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf7a8305a2b4: [TargetParser] Generate the defs for RISCV 
CPUs using llvm-tblgen. (authored by fpetrogalli).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,6 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 
 } // End llvm namespace
 
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModel"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(De

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 487962.
fpetrogalli added a comment.

Formatting changes in RISCV.td (NFC).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,6 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 
 } // End llvm namespace
 
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModel"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVTuneProcessorModel"))
+  OS << "TUNE

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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:581
+  list f = []>
+  : ProcessorModel;
+




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

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

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Gentle ping!


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 486223.
fpetrogalli added a comment.

[NFC] Restore empty line before end of namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,6 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 
 } // End llvm namespace
 
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModel"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVTuneProcessorModel"))
+ 

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 486222.
fpetrogalli added a comment.

I have addressed last round of review from @craig.topper and @pcwang-thead:

1. Renamed the RISCV-specific classes for the ProcessorModel.
2. Swapped `tunef` with `f` in `RISCVTuneProcessorModel`.
3. Fixed comment.
4. Aimed for consistency in formatting of TD files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModel"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUN

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

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:568
 
-def : ProcessorModel<"generic-rv32", NoSchedModel, [Feature32Bit]>;
-def : ProcessorModel<"generic-rv64", NoSchedModel, [Feature64Bit]>;
+class RISCVProcessorModelPROCRISCVProcessorModel?
RISCVProcessorModelTUNE_PROC->RISCVTuneProcessorModel?
I think it is a little weird that we mixed naming styles here.





Comment at: llvm/lib/Target/RISCV/RISCV.td:576
+
+class RISCVProcessorModelTUNE_PROC f,
+  list tunef = []> : 
ProcessorModel;

As for ProcessorModels for tuning, `list f` is always empty 
in both upstream and our downstream, and it is unlikely that we will specify 
target features for tune models. So it can be a default argument with value 
`[]` and swap the position of `f` and `tunef`(we are more likely to specify 
tune features). It becomes:
```
class RISCVTuneProcessorModel tunef = [],
list f = []> : ProcessorModel;
```
@craig.topper Any thoughts on this?


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

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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:572
+  string default_march = "",
+  list tunef = []> :  
ProcessorModel {
+  string DefaultMarch = default_march;

80 columns



Comment at: llvm/lib/Target/RISCV/RISCV.td:623
+  
FeatureStdExtC],
+ "rv32imafc", [TuneSiFive7]>;
+

The formatting is inconsisent. Sometimes the "rv32imafc" is on the end of the 
previous line. Can we be consistent?



Comment at: llvm/lib/TargetParser/RISCVTargetParser.cpp:1
+//===-- TargetParser - Parser for target features ---*- C++ 
-*-===//
+//

This comment should match the name of the file.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

@craig.topper @lenary @barannikov88 @pcwang-thead - New Year's gentle ping :)

Thank you,

Francesco


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 485858.
fpetrogalli added a comment.

Fix pre-merge check at 
https://buildkite.com/llvm-project/premerge-checks/builds/128468#0185724f-5ade-4603-99ce-f417efb546e8
 (missing header inclusion):

  
C:\ws\w3\llvm-project\premerge-checks\llvm\include\llvm/TargetParser/RISCVTargetParser.h(44):
 error C2039: 'vector': is not a member of 'std'
  C:\BuildTools\VC\Tools\MSVC\14.29.30133\include\string(24): note: see 
declaration of 'std'
  
C:\ws\w3\llvm-project\premerge-checks\llvm\include\llvm/TargetParser/RISCVTargetParser.h(44):
 error C2061: syntax error: identifier 'vector'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Rec.getName() << ",

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 485852.
fpetrogalli added a comment.

NFC - just an update on top of current `main`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelTUNE_

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

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

> @pcwang-thead, may I ask you to own these further optimisations of the 
> generative process, and submit a patch for it after the current patch lands? 
> I'd happily review it!
>
> The reason I am asking this is because the current patch is mostly dealing 
> with making sure we can build clang/llvm after removing the def file.  People 
> are discussing dependencies and modules (for example, last update I did was 
> to make the patch work for modules with `-DLLVM_ENABLE_MODULES=On`), and this 
> is already taking quite a number of comments.
> There is value in the discussion on how to build march out of the features, 
> I'd rather keep it in a separate submission so that the threads do not get 
> lost among the other comments for this patch.
>
> Francesco

Yes, I am happy to do it. :-)


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D137517#4012449 , @pcwang-thead 
wrote:

> In D137517#4012307 , @craig.topper 
> wrote:
>
>> In D137517#4012298 , @pcwang-thead 
>> wrote:
>>
>>> In D137517#4009175 , @fpetrogalli 
>>> wrote:
>>>
 @pcwang-thead, I addressed some of your comments.

 The value of `EnumFeatures` is now computed dynamicaly from the
 `Features` field of the `Processor` class.
>>>
>>> Thanks! That sounds great to me!
>>>
 As for generating `MArch` out of the `Features` field, @craig.topper
 pointed me at
 https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11. 
 From
 the reading of it, it seems that the alphabetical order is enough to
 build the string that carries `MArch`. Am I missing something?
>>>
>>> Currently, I think the alphabetical order is OK. If we relax the checking 
>>> of arch string someday, there is no doubt that we should change the 
>>> implementation here too.
>>
>> The currently accepted order isn’t alphabetical. The single letter 
>> extensions have a specific order. The z extensions are ordered by looking up 
>> the second letter in the single letter order. If we alphabetize here i don’t 
>> think it will be accepted by the frontend.
>
> Oops, my mistake.
>
> Here is my PoC to generate march from Features:
>
>   diff --git a/llvm/lib/Target/RISCV/RISCV.td b/llvm/lib/Target/RISCV/RISCV.td
>   index d1d0356179f5..b2520f25bfea 100644
>   --- a/llvm/lib/Target/RISCV/RISCV.td
>   +++ b/llvm/lib/Target/RISCV/RISCV.td
>   @@ -556,8 +556,8 @@ include "RISCVSchedSyntacoreSCR1.td"
>class RISCVProcessorModelPROC  SchedMachineModel m,
>  list f,
>   -  string default_march = "",
>   -  list tunef = []> :  
> ProcessorModel {
>   +  list tunef = [],
>   +  string default_march = ""> :  
> ProcessorModel {
>  string DefaultMarch = default_march;
>}
>   diff --git a/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp 
> b/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
>   index b216e82cef6c..eea31e6ddea8 100644
>   --- a/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
>   +++ b/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
>   @@ -13,17 +13,33 @@
>
>#include "TableGenBackends.h"
>#include "llvm/TableGen/Record.h"
>   +#include "llvm/Support/RISCVISAInfo.h"
>
>using namespace llvm;
>
>   -static std::string getEnumFeatures(const Record &Rec) {
>   +static int getXLen(const Record &Rec) {
>  std::vector Features = Rec.getValueAsListOfDefs("Features");
>  if (find_if(Features, [](const Record *R) {
>return R->getName() == "Feature64Bit";
>  }) != Features.end())
>   -return "FK_64BIT";
>   +return 64;
>
>   -  return "FK_NONE";
>   +  return 32;
>   +}
>   +
>   +static std::string getMArch(int XLen, const Record &Rec) {
>   +  std::vector Features = Rec.getValueAsListOfDefs("Features");
>   +  std::vector FeatureVector;
>   +  // Convert Features to FeatureVector.
>   +  for (auto *Feature : Features) {
>   +StringRef FeatureName = Feature->getValueAsString("Name");
>   +if (llvm::RISCVISAInfo::isSupportedExtensionFeature(FeatureName))
>   +  FeatureVector.push_back(std::string("+") + FeatureName.str());
>   +  }
>   +  auto ISAInfo = llvm::RISCVISAInfo::parseFeatures(XLen, FeatureVector);
>   +  if (!ISAInfo)
>   +report_fatal_error("Invalid features: ");
>   +  return (*ISAInfo)->toString();
>}
>
>void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
>   @@ -39,11 +55,17 @@ void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, 
> raw_ostream &OS) {
>  // Iterate on all definition records.
>  for (const MapTy &Def : Map) {
>const Record &Rec = *(Def.second);
>   -if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
>   +if (Rec.isSubClassOf("RISCVProcessorModelPROC")) {
>   +  int XLen = getXLen(Rec);
>   +  std::string EnumFeatures = XLen == 64 ? "FK_64BIT" : "FK_NONE";
>   +  std::string MArch = Rec.getValueAsString("DefaultMarch").str();
>   +  if (MArch == "")
>   +MArch = getMArch(XLen, Rec);
>  OS << "PROC(" << Rec.getName() << ", "
>   - << "{\"" << Rec.getValueAsString("Name") << "\"},"
>   - << getEnumFeatures(Rec) << ", "
>   - << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
>   + << "{\"" << Rec.getValueAsString("Name") << "\"}," << EnumFeatures
>   + << ", "
>   + << "{\"" << MArch << "\"})\n";
>   +}
>  }
>  OS << "\n#undef PROC\n";
>  OS << "\n";
>
> The generated file would be like below (the march strings are tedious but I 
> think that would

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484795.
fpetrogalli added a comment.

I added some fixes to make it work when building with modules 
(`-DLLVM_ENABLE_MODULES=On`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/TargetParser/CMakeLists.txt
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+

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

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D137517#4012307 , @craig.topper 
wrote:

> In D137517#4012298 , @pcwang-thead 
> wrote:
>
>> In D137517#4009175 , @fpetrogalli 
>> wrote:
>>
>>> @pcwang-thead, I addressed some of your comments.
>>>
>>> The value of `EnumFeatures` is now computed dynamicaly from the
>>> `Features` field of the `Processor` class.
>>
>> Thanks! That sounds great to me!
>>
>>> As for generating `MArch` out of the `Features` field, @craig.topper
>>> pointed me at
>>> https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11. From
>>> the reading of it, it seems that the alphabetical order is enough to
>>> build the string that carries `MArch`. Am I missing something?
>>
>> Currently, I think the alphabetical order is OK. If we relax the checking of 
>> arch string someday, there is no doubt that we should change the 
>> implementation here too.
>
> The currently accepted order isn’t alphabetical. The single letter extensions 
> have a specific order. The z extensions are ordered by looking up the second 
> letter in the single letter order. If we alphabetize here i don’t think it 
> will be accepted by the frontend.

Oops, my mistake.

Here is my PoC to generate march from Features:

  diff --git a/llvm/lib/Target/RISCV/RISCV.td b/llvm/lib/Target/RISCV/RISCV.td
  index d1d0356179f5..b2520f25bfea 100644
  --- a/llvm/lib/Target/RISCV/RISCV.td
  +++ b/llvm/lib/Target/RISCV/RISCV.td
  @@ -556,8 +556,8 @@ include "RISCVSchedSyntacoreSCR1.td"
   class RISCVProcessorModelPROC f,
  -  string default_march = "",
  -  list tunef = []> :  
ProcessorModel {
  +  list tunef = [],
  +  string default_march = ""> :  
ProcessorModel {
 string DefaultMarch = default_march;
   }
  diff --git a/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp 
b/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  index b216e82cef6c..eea31e6ddea8 100644
  --- a/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  +++ b/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  @@ -13,17 +13,33 @@
   
   #include "TableGenBackends.h"
   #include "llvm/TableGen/Record.h"
  +#include "llvm/Support/RISCVISAInfo.h"
   
   using namespace llvm;
   
  -static std::string getEnumFeatures(const Record &Rec) {
  +static int getXLen(const Record &Rec) {
 std::vector Features = Rec.getValueAsListOfDefs("Features");
 if (find_if(Features, [](const Record *R) {
   return R->getName() == "Feature64Bit";
 }) != Features.end())
  -return "FK_64BIT";
  +return 64;
   
  -  return "FK_NONE";
  +  return 32;
  +}
  +
  +static std::string getMArch(int XLen, const Record &Rec) {
  +  std::vector Features = Rec.getValueAsListOfDefs("Features");
  +  std::vector FeatureVector;
  +  // Convert Features to FeatureVector.
  +  for (auto *Feature : Features) {
  +StringRef FeatureName = Feature->getValueAsString("Name");
  +if (llvm::RISCVISAInfo::isSupportedExtensionFeature(FeatureName))
  +  FeatureVector.push_back(std::string("+") + FeatureName.str());
  +  }
  +  auto ISAInfo = llvm::RISCVISAInfo::parseFeatures(XLen, FeatureVector);
  +  if (!ISAInfo)
  +report_fatal_error("Invalid features: ");
  +  return (*ISAInfo)->toString();
   }
   
   void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
  @@ -39,11 +55,17 @@ void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, 
raw_ostream &OS) {
 // Iterate on all definition records.
 for (const MapTy &Def : Map) {
   const Record &Rec = *(Def.second);
  -if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
  +if (Rec.isSubClassOf("RISCVProcessorModelPROC")) {
  +  int XLen = getXLen(Rec);
  +  std::string EnumFeatures = XLen == 64 ? "FK_64BIT" : "FK_NONE";
  +  std::string MArch = Rec.getValueAsString("DefaultMarch").str();
  +  if (MArch == "")
  +MArch = getMArch(XLen, Rec);
 OS << "PROC(" << Rec.getName() << ", "
  - << "{\"" << Rec.getValueAsString("Name") << "\"},"
  - << getEnumFeatures(Rec) << ", "
  - << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
  + << "{\"" << Rec.getValueAsString("Name") << "\"}," << EnumFeatures
  + << ", "
  + << "{\"" << MArch << "\"})\n";
  +}
 }
 OS << "\n#undef PROC\n";
 OS << "\n";

The generated file would be like below (the march strings are tedious but I 
think that would be OK):

  #ifndef PROC
  #define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)
  #endif
  
  PROC(INVALID, {"invalid"}, FK_INVALID, {""})
  PROC(GENERIC_RV32, {"generic-rv32"},FK_NONE, {"rv32i2p0"})
  PROC(GENERIC_RV64, {"generic-rv64"},FK_64BIT, {"rv64i2p0"})
  PROC(ROCKET_RV32, {"rocket-rv32"},FK_NONE, {"rv32i2p0"})
  PROC(ROCKET_RV64, {"rocket-rv64"},FK_64BIT, {"rv64i2p0"})
  PROC(SI

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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D137517#4012298 , @pcwang-thead 
wrote:

> In D137517#4009175 , @fpetrogalli 
> wrote:
>
>> @pcwang-thead, I addressed some of your comments.
>>
>> The value of `EnumFeatures` is now computed dynamicaly from the
>> `Features` field of the `Processor` class.
>
> Thanks! That sounds great to me!
>
>> As for generating `MArch` out of the `Features` field, @craig.topper
>> pointed me at
>> https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11. From
>> the reading of it, it seems that the alphabetical order is enough to
>> build the string that carries `MArch`. Am I missing something?
>
> Currently, I think the alphabetical order is OK. If we relax the checking of 
> arch string someday, there is no doubt that we should change the 
> implementation here too.

The currently accepted order isn’t alphabetical. The single letter extensions 
have a specific order. The z extensions are ordered by looking up the second 
letter in the single letter order. If we alphabetize here i don’t think it will 
be accepted by the frontend.


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

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

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D137517#4009175 , @fpetrogalli 
wrote:

> @pcwang-thead, I addressed some of your comments.
>
> The value of `EnumFeatures` is now computed dynamicaly from the
> `Features` field of the `Processor` class.

Thanks! That sounds great to me!

> As for generating `MArch` out of the `Features` field, @craig.topper
> pointed me at
> https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11. From
> the reading of it, it seems that the alphabetical order is enough to
> build the string that carries `MArch`. Am I missing something?

Currently, I think the alphabetical order is OK. If we relax the checking of 
arch string someday, there is no doubt that we should change the implementation 
here too.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added inline comments.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:52
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))

barannikov88 wrote:
> fpetrogalli wrote:
> > barannikov88 wrote:
> > > Same for the loop above.
> > ```
> > /Users/fpetrogalli/projects/cpu-defs/upstream/llvm-project/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:38:17:
> >  error: variable 'Record' with type 'const auto *' has incompatible 
> > initializer of type 'const std::unique_ptr'
> > const auto *Record = Def.second;
> > ```
> Whoops, my bad, sorry.
> Never liked these 'auto's... Never know what is behind them. Looks like a raw 
> pointer, but it is not.
> 
I am not a fan either of `auto`. Removed.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484664.
fpetrogalli marked an inline comment as done.
fpetrogalli added a comment.

Remove all `auto` types in the emitter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,62 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(const Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](const Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  using MapTy = std::pair>;
+  using RecordMap = std::map, std::less<>>;
+  const RecordMap &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Rec.getName() << ", "
+ << "{\"" << Rec.getValueAsString("Name") << "\"},"
+ << getEnumFeatures(Rec) << ", "
+ << "{\"" << Rec.getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const MapTy &Def : Map) {
+const Record &Rec = *(Def.second);
+if (Rec.isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC(" << Rec.getName() << ", "
+ << "\"" << Rec.getValueAsString("Name") << "\")\n";
+  }
+
+  OS

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In 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 
+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 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:52
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))

fpetrogalli wrote:
> barannikov88 wrote:
> > Same for the loop above.
> ```
> /Users/fpetrogalli/projects/cpu-defs/upstream/llvm-project/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:38:17:
>  error: variable 'Record' with type 'const auto *' has incompatible 
> initializer of type 'const std::unique_ptr'
> const auto *Record = Def.second;
> ```
Whoops, my bad, sorry.
Never liked these 'auto's... Never know what is behind them. Looks like a raw 
pointer, but it is not.



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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484658.
fpetrogalli marked an inline comment as done.
fpetrogalli added a comment.

Remove `RISCVTargetParser.def` from the module map.

Follow the guidelines on  using namespace llvm 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,60 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "TableGenBackends.h"
+#include "llvm/TableGen/Record.h"
+
+using namespace llvm;
+
+static std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void llvm::EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484655.
fpetrogalli added a comment.

Restore the following...

  -const auto *Record = Def.second;
  +const auto &Record = Def.second;

... to prevent this error:

  /<...>/llvm-project/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:38:17: 
error: variable 'Record' with type 'const auto *' has incompatible initializer 
of type 'const std::unique_ptr'
  const auto *Record = Def.second;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,61 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+
+namespace llvm {
+
+static std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484654.
fpetrogalli marked an inline comment as done.
fpetrogalli added a comment.

ops - this was wrong:

  
/Users/fpetrogalli/projects/cpu-defs/upstream/llvm-project/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:38:17:
 error: variable 'Record' with type 'const auto *' has incompatible initializer 
of type 'const std::unique_ptr'
  const auto *Record = Def.second;

I'll revert it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,61 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+
+namespace llvm {
+
+static std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const auto &Def : Map) {
+const auto *Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const auto &Def : Map) {
+const auto *Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS 

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added inline comments.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:52
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))

barannikov88 wrote:
> Same for the loop above.
```
/Users/fpetrogalli/projects/cpu-defs/upstream/llvm-project/llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:38:17:
 error: variable 'Record' with type 'const auto *' has incompatible initializer 
of type 'const std::unique_ptr'
const auto *Record = Def.second;
```


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added inline comments.



Comment at: llvm/lib/TargetParser/CMakeLists.txt:29
+# LLVMTargetParser. See https://stackoverflow.com/a/25681179
+target_include_directories(LLVMTargetParser PUBLIC 
$)

barannikov88 wrote:
> Will it work if RISC-V target is not compiled-in?
> This does not strictly add a cyclic dependency, but it would still be nice to 
> avoid dependency on higher-level components.
> Is it possible / reasonable to extract the part of the RISCV.td that relates 
> to this component and put it separate td file in this directory? Or is it 
> tightly coupled with the rest of the target description?
> 
> Will it work if RISC-V target is not compiled-in?

This line worked, so yes

```
cmake ../llvm-project/llvm -DLLVM_ENABLE_PROJECTS=clang 
-DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=ON -GNinja 
-DLLVM_TARGETS_TO_BUILD="AArch64;AMDGPU;ARM;AVR;BPF;Hexagon;Lanai;Mips;MSP430;NVPTX;PowerPC;Sparc;SystemZ;VE;WebAssembly;X86;XCore"
```


> This does not strictly add a cyclic dependency, but it would still be nice to 
> avoid dependency on higher-level components.
> Is it possible / reasonable to extract the part of the RISCV.td that relates 
> to this component and put it separate td file in this directory? Or is it 
> tightly coupled with the rest of the target description?
> 

Hum - the content of RISCV.td is central in `llvm/lib/Target/RISCV/`. The only 
way I can see we can put a td file in this folder is by duplicating the content 
that we use from `RISCV.td`... That however would mean missing the point of 
this patch though, as the idea is to have centralised unique source of 
information.




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

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

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:16
+#include 
+namespace llvm {
+

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 ]].



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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added inline comments.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:15
+#include "llvm/TableGen/Record.h"
+#include 
+namespace llvm {

barannikov88 wrote:
>  [[ 
> 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 
+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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484649.
fpetrogalli marked 5 inline comments as done.
fpetrogalli added a comment.

I submitted some of the cleanup suggested by @barannikov88 - thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,61 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+
+namespace llvm {
+
+static std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (find_if(Features, [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(const RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (const auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (const auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC(" << Record->getName() << ", "
+ << "\"" << Record->getValueAsString("Name") << "\")\n";
+  }
+
+  OS << "\n#undef TUNE_PROC\n";
+}
+
+} // namespace llvm
Index: llvm/utils/TableGen/CMakeLists.txt

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

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.



Comment at: llvm/lib/TargetParser/CMakeLists.txt:29
+# LLVMTargetParser. See https://stackoverflow.com/a/25681179
+target_include_directories(LLVMTargetParser PUBLIC 
$)

Will it work if RISC-V target is not compiled-in?
This does not strictly add a cyclic dependency, but it would still be nice to 
avoid dependency on higher-level components.
Is it possible / reasonable to extract the part of the RISCV.td that relates to 
this component and put it separate td file in this directory? Or is it tightly 
coupled with the rest of the target description?




Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:15
+#include "llvm/TableGen/Record.h"
+#include 
+namespace llvm {

 [[ 
https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden | is 
forbidden ]] by the coding standards.




Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:16
+#include 
+namespace llvm {
+

This [[ 
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions
 | should be ]] `using namespace llvm;`



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:18
+
+std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");

This should be `static`.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:20
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (std::find_if(Features.begin(), Features.end(), [](Record *R) {
+return R->getName() == "Feature64Bit";

(suggestion) LLVM's version of find_if accepts ranges, which is a bit shorter.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:28
+
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();

This function does not seem to mutate RecordKeeper, so it should be `const`.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:37
+  // Iterate on all definition records.
+  for (auto &Def : Map) {
+const auto &Record = Def.second;

Should also `const`, same for the loop below.



Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:52
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))

Same for the loop above.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484564.
fpetrogalli added a comment.

Same NFC - restore an empty line change...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,60 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+#include 
+namespace llvm {
+
+std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (std::find_if(Features.begin(), Features.end(), [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC(" << Record->getName() << ", "
+ << "\"" << Record->getValueAsString("Name") << "\")\n";
+  }
+
+  OS << "\n#undef TUNE_PROC\n";
+}
+} // namespace llvm
Index: llvm/utils/TableGen/CMakeLists.txt
===
--- llvm/utils/Tab

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484563.
fpetrogalli added a comment.

NFC - I just restored an empty line to prevent modifying a file that is not 
being touched by the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,60 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+#include 
+namespace llvm {
+
+std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (std::find_if(Features.begin(), Features.end(), [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC(" << Record->getName() << ", "
+ << "\"" << Record->getValueAsString("Name") << "\")\n";
+  }
+
+  OS << "\n#undef TUNE_PROC\n";
+}
+} // namespace llvm
Index: llvm/utils/TableGen/CMake

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484561.
fpetrogalli added a comment.

There is no need to use `target_include_directories(LLVMRISCVDesc PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)` for components that are not related to the 
RISC-V target. Thank you @lenary for the suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,60 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+#include 
+namespace llvm {
+
+std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (std::find_if(Features.begin(), Features.end(), [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC(" << Record->getName() << ", "
+   

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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

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.


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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

fpetrogalli wrote:
> lenary wrote:
> > fpetrogalli wrote:
> > > craig.topper wrote:
> > > > fpetrogalli wrote:
> > > > > lenary wrote:
> > > > > > craig.topper wrote:
> > > > > > > Why do we need to touch CMake file that aren't RISC-V?
> > > > > > Yeah, this shouldn't be needed.
> > > > > > 
> > > > > > We do have some fixes for the modules build which recently landed, 
> > > > > > maybe they fix the issues you were seeing, including:
> > > > > > - 
> > > > > > https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > > > > - 
> > > > > > https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > > > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might 
> > > > > > clear up some weird things about the quicker modulemap fixes)
> > > > > This is unrelated to Modules. The .inc file generated by tablegen is 
> > > > > created in `{make_build_folder}/lib/TargetParser`. The file is then 
> > > > > included in `TargetParser.cpp` but also in `TargetParser.h` -> this 
> > > > > means that every time we include the latter in a cpp file we need to 
> > > > > make the inc file visible for inclusion.
> > > > Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
> > > Of course! :) I'll do it in a separate patch.
> > I think, if that is the case, we should add 
> > `{make_build_folder}/lib/TargetParser` as a public include directory of the 
> > LLVMTargetParser library, which should mean anything depending on it gets 
> > that as an include directory they can rely on.
> > I think, if that is the case, we should add 
> > `{make_build_folder}/lib/TargetParser` as a public include directory of the 
> > LLVMTargetParser library, which should mean anything depending on it gets 
> > that as an include directory they can rely on.
> 
> How do I do that?I tried `target_include_directories(LLVMTargetParser PUBLIC 
> ${LLVM_LIBRARY_DIR}/TargetParser/)` but I got: 
> ```
> CMake Error in lib/TargetParser/CMakeLists.txt:
>   Target "LLVMTargetParser" INTERFACE_INCLUDE_DIRECTORIES property contains
>   path:
> 
> "/Users/fpetrogalli/<...>/build-shared/./lib/TargetParser/"
> 
>   which is prefixed in the build directory.
> 
> ```
I think https://stackoverflow.com/a/25681179 points to how to do this in such a 
way that it works properly for people using cmake install. 


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a subscriber: aprantl.
fpetrogalli added inline comments.



Comment at: llvm/include/llvm/module.modulemap:420
 textual header "Support/CSKYTargetParser.def"
-textual header "Support/RISCVTargetParser.def"
 textual header "Support/TargetOpcodes.def"

@aprantl - I have removed a `def` file that was needed to build part of the 
interface of the `TargetParser` in `LLVMSupport`. I suspect this might have 
consequences on the Modules? The correspondent file is now created in the build 
folder as `lib/TargetParser/RISCVTargetParserDef.inc`.

Just giving you heads up in case any of the builders that use modules start 
failing once this code lands in `main`.


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added inline comments.



Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

lenary wrote:
> fpetrogalli wrote:
> > craig.topper wrote:
> > > fpetrogalli wrote:
> > > > lenary wrote:
> > > > > craig.topper wrote:
> > > > > > Why do we need to touch CMake file that aren't RISC-V?
> > > > > Yeah, this shouldn't be needed.
> > > > > 
> > > > > We do have some fixes for the modules build which recently landed, 
> > > > > maybe they fix the issues you were seeing, including:
> > > > > - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > > > - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might 
> > > > > clear up some weird things about the quicker modulemap fixes)
> > > > This is unrelated to Modules. The .inc file generated by tablegen is 
> > > > created in `{make_build_folder}/lib/TargetParser`. The file is then 
> > > > included in `TargetParser.cpp` but also in `TargetParser.h` -> this 
> > > > means that every time we include the latter in a cpp file we need to 
> > > > make the inc file visible for inclusion.
> > > Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
> > Of course! :) I'll do it in a separate patch.
> I think, if that is the case, we should add 
> `{make_build_folder}/lib/TargetParser` as a public include directory of the 
> LLVMTargetParser library, which should mean anything depending on it gets 
> that as an include directory they can rely on.
> I think, if that is the case, we should add 
> `{make_build_folder}/lib/TargetParser` as a public include directory of the 
> LLVMTargetParser library, which should mean anything depending on it gets 
> that as an include directory they can rely on.

How do I do that?I tried `target_include_directories(LLVMTargetParser PUBLIC 
${LLVM_LIBRARY_DIR}/TargetParser/)` but I got: 
```
CMake Error in lib/TargetParser/CMakeLists.txt:
  Target "LLVMTargetParser" INTERFACE_INCLUDE_DIRECTORIES property contains
  path:

"/Users/fpetrogalli/<...>/build-shared/./lib/TargetParser/"

  which is prefixed in the build directory.

```



Comment at: llvm/lib/Target/RISCV/CMakeLists.txt:69
+  DEPENDS
+  LLVMTargetParser
   )

lenary wrote:
> Why is this needed? I already added TargetParser to the list of required 
> components, on (new) line 61, is this doing something else?
My bad - removed!


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484391.
fpetrogalli added a comment.

@pcwang-thead, I addressed some of your comments.

The value of `EnumFeatures` is now computed dynamicaly from the
`Features` field of the `Processor` class.

As for generating `MArch` out of the `Features` field, @craig.topper
pointed me at
https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/11. From
the reading of it, it seems that the alphabetical order is enough to
build the string that carries `MArch`. Am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt
  llvm/lib/Target/RISCV/CMakeLists.txt
  llvm/lib/Target/RISCV/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/unittests/Target/AMDGPU/CMakeLists.txt
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,60 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+#include 
+namespace llvm {
+
+std::string getEnumFeatures(Record &Rec) {
+  std::vector Features = Rec.getValueAsListOfDefs("Features");
+  if (std::find_if(Features.begin(), Features.end(), [](Record *R) {
+return R->getName() == "Feature64Bit";
+  }) != Features.end())
+return "FK_64BIT";
+
+  return "FK_NONE";
+}
+
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << getEnumFeatures(*Record) << ", "
+   

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 484380.
fpetrogalli marked 8 inline comments as done.
fpetrogalli added a comment.

1. I have split the RISCV bits of the target parser out of TargetParser.cpp so 
that none of the non-RISCV components need to include

the folder with the autogenerated .inc file.

2. I have removed the hard-coded RISCVTargetParser.def
3. Minor cosmetic changes requested from review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

Files:
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.def
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/include/llvm/module.modulemap
  llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt
  llvm/lib/Target/RISCV/CMakeLists.txt
  llvm/lib/Target/RISCV/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/TargetParser/CMakeLists.txt
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/lib/TargetParser/TargetParser.cpp
  llvm/unittests/Target/AMDGPU/CMakeLists.txt
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
  llvm/utils/TableGen/TableGen.cpp
  llvm/utils/TableGen/TableGenBackends.h

Index: llvm/utils/TableGen/TableGenBackends.h
===
--- llvm/utils/TableGen/TableGenBackends.h
+++ llvm/utils/TableGen/TableGenBackends.h
@@ -94,7 +94,7 @@
 void EmitDirectivesDecl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDirectivesImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitDXILOperation(RecordKeeper &RK, raw_ostream &OS);
-
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS);
 } // End llvm namespace
 
 #endif
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -58,6 +58,7 @@
   GenDirectivesEnumDecl,
   GenDirectivesEnumImpl,
   GenDXILOperation,
+  GenRISCVTargetDef,
 };
 
 namespace llvm {
@@ -141,8 +142,9 @@
 clEnumValN(GenDirectivesEnumImpl, "gen-directive-impl",
"Generate directive related implementation code"),
 clEnumValN(GenDXILOperation, "gen-dxil-operation",
-   "Generate DXIL operation information")));
-
+   "Generate DXIL operation information"),
+clEnumValN(GenRISCVTargetDef, "gen-riscv-target-def",
+   "Generate the list of CPU for RISCV")));
 cl::OptionCategory PrintEnumsCat("Options for -print-enums");
 cl::opt Class("class", cl::desc("Print Enum list for this class"),
cl::value_desc("class name"),
@@ -278,6 +280,9 @@
   case GenDXILOperation:
 EmitDXILOperation(Records, OS);
 break;
+  case GenRISCVTargetDef:
+EmitRISCVTargetDef(Records, OS);
+break;
   }
 
   return false;
Index: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
===
--- /dev/null
+++ llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
@@ -0,0 +1,49 @@
+//===- RISCVTargetDefEmitter.cpp - Generate lists of RISCV CPUs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tablegen backend emits the include file needed by the target
+// parser to parse the RISC-V CPUs.
+//
+//===--===//
+
+#include "llvm/TableGen/Record.h"
+
+namespace llvm {
+void EmitRISCVTargetDef(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Map = RK.getDefs();
+
+  OS << "#ifndef PROC\n"
+ << "#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)\n"
+ << "#endif\n\n";
+
+  OS << "PROC(INVALID, {\"invalid\"}, FK_INVALID, {\"\"})\n";
+  // Iterate on all definition records.
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelPROC"))
+  OS << "PROC(" << Record->getName() << ", "
+ << "{\"" << Record->getValueAsString("Name") << "\"},"
+ << Record->getValueAsString("EnumFeatures") << ", "
+ << "{\"" << Record->getValueAsString("DefaultMarch") << "\"})\n";
+  }
+  OS << "\n#undef PROC\n";
+  OS << "\n";
+  OS << "#ifndef TUNE_PROC\n"
+ << "#define TUNE_PROC(ENUM, NAME)\n"
+ << "#endif\n\n";
+  OS << "TUNE_PROC(GENERIC, \"generic\")\n";
+  for (auto &Def : Map) {
+const auto &Record = Def.second;
+if (Record->isSubClassOf("RISCVProcessorModelTUNE_PROC"))
+  OS << "TUNE_PROC(" << Record

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

fpetrogalli wrote:
> craig.topper wrote:
> > fpetrogalli wrote:
> > > lenary wrote:
> > > > craig.topper wrote:
> > > > > Why do we need to touch CMake file that aren't RISC-V?
> > > > Yeah, this shouldn't be needed.
> > > > 
> > > > We do have some fixes for the modules build which recently landed, 
> > > > maybe they fix the issues you were seeing, including:
> > > > - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > > - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might 
> > > > clear up some weird things about the quicker modulemap fixes)
> > > This is unrelated to Modules. The .inc file generated by tablegen is 
> > > created in `{make_build_folder}/lib/TargetParser`. The file is then 
> > > included in `TargetParser.cpp` but also in `TargetParser.h` -> this means 
> > > that every time we include the latter in a cpp file we need to make the 
> > > inc file visible for inclusion.
> > Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
> Of course! :) I'll do it in a separate patch.
I think, if that is the case, we should add 
`{make_build_folder}/lib/TargetParser` as a public include directory of the 
LLVMTargetParser library, which should mean anything depending on it gets that 
as an include directory they can rely on.



Comment at: llvm/lib/Target/RISCV/CMakeLists.txt:69
+  DEPENDS
+  LLVMTargetParser
   )

Why is this needed? I already added TargetParser to the list of required 
components, on (new) line 61, is this doing something else?


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added inline comments.



Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

craig.topper wrote:
> fpetrogalli wrote:
> > lenary wrote:
> > > craig.topper wrote:
> > > > Why do we need to touch CMake file that aren't RISC-V?
> > > Yeah, this shouldn't be needed.
> > > 
> > > We do have some fixes for the modules build which recently landed, maybe 
> > > they fix the issues you were seeing, including:
> > > - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > > - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > > - https://reviews.llvm.org/D140420 (this isn't landed, but it might clear 
> > > up some weird things about the quicker modulemap fixes)
> > This is unrelated to Modules. The .inc file generated by tablegen is 
> > created in `{make_build_folder}/lib/TargetParser`. The file is then 
> > included in `TargetParser.cpp` but also in `TargetParser.h` -> this means 
> > that every time we include the latter in a cpp file we need to make the inc 
> > file visible for inclusion.
> Can we split RISC-V out of TargetParser.cpp and TargetParser.h?
Of course! :) I'll do it in a separate patch.


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

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

[Craig Topper via Phabricator via cfe-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/)

fpetrogalli wrote:
> lenary wrote:
> > craig.topper wrote:
> > > Why do we need to touch CMake file that aren't RISC-V?
> > Yeah, this shouldn't be needed.
> > 
> > We do have some fixes for the modules build which recently landed, maybe 
> > they fix the issues you were seeing, including:
> > - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> > - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> > - https://reviews.llvm.org/D140420 (this isn't landed, but it might clear 
> > up some weird things about the quicker modulemap fixes)
> This is unrelated to Modules. The .inc file generated by tablegen is created 
> in `{make_build_folder}/lib/TargetParser`. The file is then included in 
> `TargetParser.cpp` but also in `TargetParser.h` -> this means that every time 
> we include the latter in a cpp file we need to make the inc file visible for 
> inclusion.
Can we split RISC-V out of TargetParser.cpp and TargetParser.h?


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

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

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Hi - gentle ping on reviewing this




Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

lenary wrote:
> craig.topper wrote:
> > Why do we need to touch CMake file that aren't RISC-V?
> Yeah, this shouldn't be needed.
> 
> We do have some fixes for the modules build which recently landed, maybe they 
> fix the issues you were seeing, including:
> - https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
> - https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
> - https://reviews.llvm.org/D140420 (this isn't landed, but it might clear up 
> some weird things about the quicker modulemap fixes)
This is unrelated to Modules. The .inc file generated by tablegen is created in 
`{make_build_folder}/lib/TargetParser`. The file is then included in 
`TargetParser.cpp` but also in `TargetParser.h` -> this means that every time 
we include the latter in a cpp file we need to make the inc file visible for 
inclusion.


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

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

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

One comment on the build changes, I don't have opinions on the RISC-V changes.




Comment at: llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt:19
 
+target_include_directories(LLVMAArch64AsmParser PRIVATE 
${LLVM_LIBRARY_DIR}/TargetParser/)

craig.topper wrote:
> Why do we need to touch CMake file that aren't RISC-V?
Yeah, this shouldn't be needed.

We do have some fixes for the modules build which recently landed, maybe they 
fix the issues you were seeing, including:
- https://reviews.llvm.org/rG9cd6fbee7ed881f8e80b735e95567040e56f189e
- https://reviews.llvm.org/rG6bdf378dcd349d97152846bb687c1d1de511d138
- https://reviews.llvm.org/D140420 (this isn't landed, but it might clear up 
some weird things about the quicker modulemap fixes)


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

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

[Craig Topper via Phabricator via cfe-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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits