Skip to content

[RISCV] Flatten the ImpliedExts table in RISCVISAInfo.cpp #89975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 26, 2024
Merged

[RISCV] Flatten the ImpliedExts table in RISCVISAInfo.cpp #89975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66d4815
[RISCV] Flatten the ImpliedExts table in RISCVISAInfo.cpp
topperc Apr 24, 2024
c43ad2a
fixup! Use std::for_each.
topperc Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 40 additions & 41 deletions llvm/lib/TargetParser/RISCVISAInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,15 +847,21 @@ Error RISCVISAInfo::checkDependency() {

struct ImpliedExtsEntry {
StringLiteral Name;
ArrayRef<const char *> Exts;
const char *ImpliedExt;

bool operator<(const ImpliedExtsEntry &Other) const {
return Name < Other.Name;
}

bool operator<(StringRef Other) const { return Name < Other; }
};

static bool operator<(const ImpliedExtsEntry &LHS, StringRef RHS) {
return LHS.Name < RHS;
}

static bool operator<(StringRef LHS, const ImpliedExtsEntry &RHS) {
return LHS < RHS.Name;
}

#define GET_IMPLIED_EXTENSIONS
#include "llvm/TargetParser/RISCVTargetParserDef.inc"

Expand All @@ -880,18 +886,19 @@ void RISCVISAInfo::updateImplication() {

while (!WorkList.empty()) {
StringRef ExtName = WorkList.pop_back_val();
auto I = llvm::lower_bound(ImpliedExts, ExtName);
if (I != std::end(ImpliedExts) && I->Name == ExtName) {
for (const char *ImpliedExt : I->Exts) {
if (WorkList.count(ImpliedExt))
continue;
if (Exts.count(ImpliedExt))
continue;
auto Version = findDefaultVersion(ImpliedExt);
addExtension(ImpliedExt, Version.value());
WorkList.insert(ImpliedExt);
}
}
auto Range = std::equal_range(std::begin(ImpliedExts),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have llvm::equal_range wrapper? Maybe we should add it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of wrappers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like other wrappers in STLExtras.h, we can save our lives. :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I'm just curious about the advantages of those llvm wrappers.
I was just thinking whether I should use the wrapper or std every time lol~

std::end(ImpliedExts), ExtName);
std::for_each(Range.first, Range.second,
[&](const ImpliedExtsEntry &Implied) {
const char *ImpliedExt = Implied.ImpliedExt;
if (WorkList.count(ImpliedExt))
return;
if (Exts.count(ImpliedExt))
return;
auto Version = findDefaultVersion(ImpliedExt);
addExtension(ImpliedExt, Version.value());
WorkList.insert(ImpliedExt);
});
}

// Add Zcf if Zce and F are enabled on RV32.
Expand All @@ -902,42 +909,34 @@ void RISCVISAInfo::updateImplication() {
}
}

struct CombinedExtsEntry {
StringLiteral CombineExt;
ArrayRef<const char *> RequiredExts;
};

static constexpr CombinedExtsEntry CombineIntoExts[] = {
{{"zk"}, {ImpliedExtsZk}},
{{"zkn"}, {ImpliedExtsZkn}},
{{"zks"}, {ImpliedExtsZks}},
{{"zvkn"}, {ImpliedExtsZvkn}},
{{"zvknc"}, {ImpliedExtsZvknc}},
{{"zvkng"}, {ImpliedExtsZvkng}},
{{"zvks"}, {ImpliedExtsZvks}},
{{"zvksc"}, {ImpliedExtsZvksc}},
{{"zvksg"}, {ImpliedExtsZvksg}},
static constexpr StringLiteral CombineIntoExts[] = {
{"zk"}, {"zkn"}, {"zks"}, {"zvkn"}, {"zvknc"},
{"zvkng"}, {"zvks"}, {"zvksc"}, {"zvksg"},
};

void RISCVISAInfo::updateCombination() {
bool IsNewCombine = false;
bool MadeChange = false;
do {
IsNewCombine = false;
for (CombinedExtsEntry CombineIntoExt : CombineIntoExts) {
auto CombineExt = CombineIntoExt.CombineExt;
auto RequiredExts = CombineIntoExt.RequiredExts;
MadeChange = false;
for (StringRef CombineExt : CombineIntoExts) {
if (hasExtension(CombineExt))
continue;
bool IsAllRequiredFeatureExist = true;
for (const char *Ext : RequiredExts)
IsAllRequiredFeatureExist &= hasExtension(Ext);
if (IsAllRequiredFeatureExist) {

// Look up the extension in the ImpliesExt table to find everything it
// depends on.
auto Range = std::equal_range(std::begin(ImpliedExts),
std::end(ImpliedExts), CombineExt);
bool HasAllRequiredFeatures = std::all_of(
Range.first, Range.second, [&](const ImpliedExtsEntry &Implied) {
return hasExtension(Implied.ImpliedExt);
});
if (HasAllRequiredFeatures) {
auto Version = findDefaultVersion(CombineExt);
addExtension(CombineExt, Version.value());
IsNewCombine = true;
MadeChange = true;
}
}
} while (IsNewCombine);
} while (MadeChange);
}

void RISCVISAInfo::updateFLen() {
Expand Down
4 changes: 1 addition & 3 deletions llvm/test/TableGen/riscv-target-def.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,8 @@ def ROCKET : RISCVTuneProcessorModel<"rocket",
// CHECK: #ifdef GET_IMPLIED_EXTENSIONS
// CHECK-NEXT: #undef GET_IMPLIED_EXTENSIONS

// CHECK: static const char *ImpliedExtsF[] = {"zicsr"};

// CHECK: static constexpr ImpliedExtsEntry ImpliedExts[] = {
// CHECK-NEXT: { {"f"}, {ImpliedExtsF} },
// CHECK-NEXT: { {"f"}, "zicsr"},
// CHECK-NEXT: };

// CHECK: #endif // GET_IMPLIED_EXTENSIONS
Expand Down
30 changes: 4 additions & 26 deletions llvm/utils/TableGen/RISCVTargetDefEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,6 @@ static void printExtensionTable(raw_ostream &OS,
OS << "};\n\n";
}

// Get the extension name from the Record name. This gives the canonical
// capitalization.
static StringRef getExtensionNameFromRecordName(const Record *R) {
StringRef Name = R->getName();
if (!Name.consume_front("FeatureStdExt"))
Name.consume_front("FeatureVendor");

return Name;
}

static void emitRISCVExtensions(RecordKeeper &Records, raw_ostream &OS) {
OS << "#ifdef GET_SUPPORTED_EXTENSIONS\n";
OS << "#undef GET_SUPPORTED_EXTENSIONS\n\n";
Expand All @@ -71,33 +61,21 @@ static void emitRISCVExtensions(RecordKeeper &Records, raw_ostream &OS) {
OS << "#ifdef GET_IMPLIED_EXTENSIONS\n";
OS << "#undef GET_IMPLIED_EXTENSIONS\n\n";

OS << "\nstatic constexpr ImpliedExtsEntry ImpliedExts[] = {\n";
for (Record *Ext : Extensions) {
auto ImpliesList = Ext->getValueAsListOfDefs("Implies");
if (ImpliesList.empty())
continue;

OS << "static const char *ImpliedExts"
<< getExtensionNameFromRecordName(Ext) << "[] = {";
StringRef Name = getExtensionName(Ext);

ListSeparator LS(", ");
for (auto *ImpliedExt : ImpliesList) {
if (!ImpliedExt->isSubClassOf("RISCVExtension"))
continue;

OS << LS << '"' << getExtensionName(ImpliedExt) << '"';
OS << " { {\"" << Name << "\"}, \"" << getExtensionName(ImpliedExt)
<< "\"},\n";
}

OS << "};\n";
}

OS << "\nstatic constexpr ImpliedExtsEntry ImpliedExts[] = {\n";
for (Record *Ext : Extensions) {
auto ImpliesList = Ext->getValueAsListOfDefs("Implies");
if (ImpliesList.empty())
continue;

OS << " { {\"" << getExtensionName(Ext) << "\"}, {ImpliedExts"
<< getExtensionNameFromRecordName(Ext) << "} },\n";
}

OS << "};\n\n";
Expand Down