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

Thu Dec 22 00:11:27 PST 2022

pcwang-thead added a comment.

In D137517#4012307 <https://reviews.llvm.org/D137517#4012307>, @craig.topper wrote:

> In D137517#4012298 <https://reviews.llvm.org/D137517#4012298>, @pcwang-thead wrote:
>
>> In D137517#4009175 <https://reviews.llvm.org/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<string n,
                                 SchedMachineModel m,
                                 list<SubtargetFeature> f,
  -                              string default_march = "",
  -                              list<SubtargetFeature> tunef = []> :  ProcessorModel<n, m, f, tunef> {
  +                              list<SubtargetFeature> tunef = [],
  +                              string default_march = ""> :  ProcessorModel<n, m, f, tunef> {
     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<Record *> 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<Record *> Features = Rec.getValueAsListOfDefs("Features");
  +  std::vector<std::string> 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(SIFIVE_E20, {"sifive-e20"},FK_NONE, {"rv32i2p0_m2p0_c2p0"})
  PROC(SIFIVE_E21, {"sifive-e21"},FK_NONE, {"rv32i2p0_m2p0_a2p0_c2p0"})
  PROC(SIFIVE_E24, {"sifive-e24"},FK_NONE, {"rv32i2p0_m2p0_a2p0_f2p0_c2p0"})
  PROC(SIFIVE_E31, {"sifive-e31"},FK_NONE, {"rv32i2p0_m2p0_a2p0_c2p0"})
  PROC(SIFIVE_E34, {"sifive-e34"},FK_NONE, {"rv32i2p0_m2p0_a2p0_f2p0_c2p0"})
  PROC(SIFIVE_E76, {"sifive-e76"},FK_NONE, {"rv32i2p0_m2p0_a2p0_f2p0_c2p0"})
  PROC(SIFIVE_S21, {"sifive-s21"},FK_64BIT, {"rv64i2p0_m2p0_a2p0_c2p0"})
  PROC(SIFIVE_S51, {"sifive-s51"},FK_64BIT, {"rv64i2p0_m2p0_a2p0_c2p0"})
  PROC(SIFIVE_S54, {"sifive-s54"},FK_64BIT, {"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"})
  PROC(SIFIVE_S76, {"sifive-s76"},FK_64BIT, {"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"})
  PROC(SIFIVE_U54, {"sifive-u54"},FK_64BIT, {"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"})
  PROC(SIFIVE_U74, {"sifive-u74"},FK_64BIT, {"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"})
  PROC(SYNTACORE_SCR1_BASE, {"syntacore-scr1-base"},FK_NONE, {"rv32i2p0_c2p0"})
  PROC(SYNTACORE_SCR1_MAX, {"syntacore-scr1-max"},FK_NONE, {"rv32i2p0_m2p0_c2p0"})

  #undef PROC

  #ifndef TUNE_PROC
  #define TUNE_PROC(ENUM, NAME)
  #endif

  TUNE_PROC(GENERIC, "generic")
  TUNE_PROC(ROCKET, "rocket")
  TUNE_PROC(SIFIVE_7, "sifive-7-series")

  #undef TUNE_PROC

The key point is building `RISCVISAInfo` from feature vectors and using `RISCVISAInfo::toString()` to construct the march string. If there is no default march string, then we can generate it from CPU's features.

This will cause a cyclic dependency: tablengen->TargetParser->tablengen, so I move `RISCVISAInfo.cpp` back to `Support` component in D140529 <https://reviews.llvm.org/D140529>.

Is it OK with you? @craig.topper

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517