[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.
Francesco Petrogalli via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 22 04:26:06 PST 2022
fpetrogalli added a comment.
In D137517#4012449 <https://reviews.llvm.org/D137517#4012449>, @pcwang-thead wrote:
> 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
@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
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137517/new/
https://reviews.llvm.org/D137517
More information about the llvm-commits
mailing list