Skip to content

[RISCV] Assert extensions are sorted at compile time. NFCI #77442

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

Closed
wants to merge 2 commits into from
Closed

[RISCV] Assert extensions are sorted at compile time. NFCI #77442

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff7a9ce
[RISCV] Assert extensions are sorted at compile time. NFCI
lukel97 Jan 9, 2024
4b1d167
Use the string literal argument in static_assert
lukel97 Jan 9, 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
162 changes: 80 additions & 82 deletions llvm/lib/Support/RISCVISAInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "llvm/Support/raw_ostream.h"

#include <array>
#include <atomic>
#include <optional>
#include <string>
#include <vector>
Expand All @@ -35,8 +34,8 @@ struct RISCVSupportedExtension {
/// Supported version.
RISCVExtensionVersion Version;

bool operator<(const RISCVSupportedExtension &RHS) const {
return StringRef(Name) < StringRef(RHS.Name);
constexpr bool operator<(const RISCVSupportedExtension &RHS) const {
return std::string_view(Name) < std::string_view(RHS.Name);
}
};

Expand All @@ -49,7 +48,7 @@ static const char *RISCVGImplications[] = {
};

// NOTE: This table should be sorted alphabetically by extension name.
static const RISCVSupportedExtension SupportedExtensions[] = {
static constexpr RISCVSupportedExtension SupportedExtensions[] = {
{"a", RISCVExtensionVersion{2, 1}},
{"c", RISCVExtensionVersion{2, 0}},
{"d", RISCVExtensionVersion{2, 2}},
Expand Down Expand Up @@ -187,7 +186,7 @@ static const RISCVSupportedExtension SupportedExtensions[] = {
};

// NOTE: This table should be sorted alphabetically by extension name.
static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
static constexpr RISCVSupportedExtension SupportedExperimentalExtensions[] = {
{"zacas", RISCVExtensionVersion{1, 0}},

{"zcmop", RISCVExtensionVersion{0, 2}},
Expand All @@ -207,19 +206,19 @@ static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
{"zvfbfwma", RISCVExtensionVersion{0, 8}},
};

static void verifyTables() {
#ifndef NDEBUG
static std::atomic<bool> TableChecked(false);
if (!TableChecked.load(std::memory_order_relaxed)) {
assert(llvm::is_sorted(SupportedExtensions) &&
"Extensions are not sorted by name");
assert(llvm::is_sorted(SupportedExperimentalExtensions) &&
"Experimental extensions are not sorted by name");
TableChecked.store(true, std::memory_order_relaxed);
}
#endif
// TODO: Replace with std::is_sorted once we move to C++20
template <typename T> constexpr bool isSorted(T &&Extensions) {
for (size_t I = 0; I < std::size(Extensions) - 1; I++)
if (!(Extensions[I] < Extensions[I + 1]))
return false;
return true;
}

static_assert(isSorted(SupportedExtensions),
"Extensions are not sorted by name");
static_assert(isSorted(SupportedExperimentalExtensions),
"Experimental extensions are not sorted by name");

static void PrintExtension(StringRef Name, StringRef Version,
StringRef Description) {
outs().indent(4);
Expand Down Expand Up @@ -359,8 +358,6 @@ bool RISCVISAInfo::isSupportedExtensionFeature(StringRef Ext) {
}

bool RISCVISAInfo::isSupportedExtension(StringRef Ext) {
verifyTables();

for (auto ExtInfo : {ArrayRef(SupportedExtensions),
ArrayRef(SupportedExperimentalExtensions)}) {
auto I = llvm::lower_bound(ExtInfo, Ext, LessExtName());
Expand Down Expand Up @@ -1062,79 +1059,82 @@ static const char *ImpliedExtsZvl65536b[] = {"zvl32768b"};
static const char *ImpliedExtsZvl8192b[] = {"zvl4096b"};

struct ImpliedExtsEntry {
StringLiteral Name;
const char *Name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this have compile time impact? Now we have to do a strlen call every time we search this table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this because StringLiteral didn't have a constexpr < operator. But I took another stab at this, and I think we can actually make the StringLiteral std::string_view operator constexpr.

ArrayRef<const char *> Exts;

bool operator<(const ImpliedExtsEntry &Other) const {
return Name < Other.Name;
constexpr bool operator<(const ImpliedExtsEntry &Other) const {
return std::string_view(Name) < std::string_view(Other.Name);
}

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

// Note: The table needs to be sorted by name.
static constexpr ImpliedExtsEntry ImpliedExts[] = {
{{"d"}, {ImpliedExtsD}},
{{"f"}, {ImpliedExtsF}},
{{"v"}, {ImpliedExtsV}},
{{"xsfvcp"}, {ImpliedExtsXSfvcp}},
{{"xsfvfnrclipxfqf"}, {ImpliedExtsXSfvfnrclipxfqf}},
{{"xsfvfwmaccqqq"}, {ImpliedExtsXSfvfwmaccqqq}},
{{"xsfvqmaccdod"}, {ImpliedExtsXSfvqmaccdod}},
{{"xsfvqmaccqoq"}, {ImpliedExtsXSfvqmaccqoq}},
{{"xtheadvdot"}, {ImpliedExtsXTHeadVdot}},
{{"zacas"}, {ImpliedExtsZacas}},
{{"zcb"}, {ImpliedExtsZcb}},
{{"zcd"}, {ImpliedExtsZcd}},
{{"zce"}, {ImpliedExtsZce}},
{{"zcf"}, {ImpliedExtsZcf}},
{{"zcmop"}, {ImpliedExtsZcmop}},
{{"zcmp"}, {ImpliedExtsZcmp}},
{{"zcmt"}, {ImpliedExtsZcmt}},
{{"zdinx"}, {ImpliedExtsZdinx}},
{{"zfa"}, {ImpliedExtsZfa}},
{{"zfbfmin"}, {ImpliedExtsZfbfmin}},
{{"zfh"}, {ImpliedExtsZfh}},
{{"zfhmin"}, {ImpliedExtsZfhmin}},
{{"zfinx"}, {ImpliedExtsZfinx}},
{{"zhinx"}, {ImpliedExtsZhinx}},
{{"zhinxmin"}, {ImpliedExtsZhinxmin}},
{{"zicfiss"}, {ImpliedExtsZicfiss}},
{{"zicntr"}, {ImpliedExtsZicntr}},
{{"zihpm"}, {ImpliedExtsZihpm}},
{{"zk"}, {ImpliedExtsZk}},
{{"zkn"}, {ImpliedExtsZkn}},
{{"zks"}, {ImpliedExtsZks}},
{{"zvbb"}, {ImpliedExtsZvbb}},
{{"zve32f"}, {ImpliedExtsZve32f}},
{{"zve32x"}, {ImpliedExtsZve32x}},
{{"zve64d"}, {ImpliedExtsZve64d}},
{{"zve64f"}, {ImpliedExtsZve64f}},
{{"zve64x"}, {ImpliedExtsZve64x}},
{{"zvfbfmin"}, {ImpliedExtsZvfbfmin}},
{{"zvfbfwma"}, {ImpliedExtsZvfbfwma}},
{{"zvfh"}, {ImpliedExtsZvfh}},
{{"zvfhmin"}, {ImpliedExtsZvfhmin}},
{{"zvkn"}, {ImpliedExtsZvkn}},
{{"zvknc"}, {ImpliedExtsZvknc}},
{{"zvkng"}, {ImpliedExtsZvkng}},
{{"zvknhb"}, {ImpliedExtsZvknhb}},
{{"zvks"}, {ImpliedExtsZvks}},
{{"zvksc"}, {ImpliedExtsZvksc}},
{{"zvksg"}, {ImpliedExtsZvksg}},
{{"zvl1024b"}, {ImpliedExtsZvl1024b}},
{{"zvl128b"}, {ImpliedExtsZvl128b}},
{{"zvl16384b"}, {ImpliedExtsZvl16384b}},
{{"zvl2048b"}, {ImpliedExtsZvl2048b}},
{{"zvl256b"}, {ImpliedExtsZvl256b}},
{{"zvl32768b"}, {ImpliedExtsZvl32768b}},
{{"zvl4096b"}, {ImpliedExtsZvl4096b}},
{{"zvl512b"}, {ImpliedExtsZvl512b}},
{{"zvl64b"}, {ImpliedExtsZvl64b}},
{{"zvl65536b"}, {ImpliedExtsZvl65536b}},
{{"zvl8192b"}, {ImpliedExtsZvl8192b}},
{"d", {ImpliedExtsD}},
{"f", {ImpliedExtsF}},
{"v", {ImpliedExtsV}},
{"xsfvcp", {ImpliedExtsXSfvcp}},
{"xsfvfnrclipxfqf", {ImpliedExtsXSfvfnrclipxfqf}},
{"xsfvfwmaccqqq", {ImpliedExtsXSfvfwmaccqqq}},
{"xsfvqmaccdod", {ImpliedExtsXSfvqmaccdod}},
{"xsfvqmaccqoq", {ImpliedExtsXSfvqmaccqoq}},
{"xtheadvdot", {ImpliedExtsXTHeadVdot}},
{"zacas", {ImpliedExtsZacas}},
{"zcb", {ImpliedExtsZcb}},
{"zcd", {ImpliedExtsZcd}},
{"zce", {ImpliedExtsZce}},
{"zcf", {ImpliedExtsZcf}},
{"zcmop", {ImpliedExtsZcmop}},
{"zcmp", {ImpliedExtsZcmp}},
{"zcmt", {ImpliedExtsZcmt}},
{"zdinx", {ImpliedExtsZdinx}},
{"zfa", {ImpliedExtsZfa}},
{"zfbfmin", {ImpliedExtsZfbfmin}},
{"zfh", {ImpliedExtsZfh}},
{"zfhmin", {ImpliedExtsZfhmin}},
{"zfinx", {ImpliedExtsZfinx}},
{"zhinx", {ImpliedExtsZhinx}},
{"zhinxmin", {ImpliedExtsZhinxmin}},
{"zicfiss", {ImpliedExtsZicfiss}},
{"zicntr", {ImpliedExtsZicntr}},
{"zihpm", {ImpliedExtsZihpm}},
{"zk", {ImpliedExtsZk}},
{"zkn", {ImpliedExtsZkn}},
{"zks", {ImpliedExtsZks}},
{"zvbb", {ImpliedExtsZvbb}},
{"zve32f", {ImpliedExtsZve32f}},
{"zve32x", {ImpliedExtsZve32x}},
{"zve64d", {ImpliedExtsZve64d}},
{"zve64f", {ImpliedExtsZve64f}},
{"zve64x", {ImpliedExtsZve64x}},
{"zvfbfmin", {ImpliedExtsZvfbfmin}},
{"zvfbfwma", {ImpliedExtsZvfbfwma}},
{"zvfh", {ImpliedExtsZvfh}},
{"zvfhmin", {ImpliedExtsZvfhmin}},
{"zvkn", {ImpliedExtsZvkn}},
{"zvknc", {ImpliedExtsZvknc}},
{"zvkng", {ImpliedExtsZvkng}},
{"zvknhb", {ImpliedExtsZvknhb}},
{"zvks", {ImpliedExtsZvks}},
{"zvksc", {ImpliedExtsZvksc}},
{"zvksg", {ImpliedExtsZvksg}},
{"zvl1024b", {ImpliedExtsZvl1024b}},
{"zvl128b", {ImpliedExtsZvl128b}},
{"zvl16384b", {ImpliedExtsZvl16384b}},
{"zvl2048b", {ImpliedExtsZvl2048b}},
{"zvl256b", {ImpliedExtsZvl256b}},
{"zvl32768b", {ImpliedExtsZvl32768b}},
{"zvl4096b", {ImpliedExtsZvl4096b}},
{"zvl512b", {ImpliedExtsZvl512b}},
{"zvl64b", {ImpliedExtsZvl64b}},
{"zvl65536b", {ImpliedExtsZvl65536b}},
{"zvl8192b", {ImpliedExtsZvl8192b}},
};

static_assert(isSorted(ImpliedExts),
"Implied extensions are not sorted by name");

void RISCVISAInfo::updateImplication() {
bool HasE = Exts.count("e") != 0;
bool HasI = Exts.count("i") != 0;
Expand All @@ -1146,8 +1146,6 @@ void RISCVISAInfo::updateImplication() {
addExtension("i", Version->Major, Version->Minor);
}

assert(llvm::is_sorted(ImpliedExts) && "Table not sorted by Name");

// This loop may execute over 1 iteration since implication can be layered
// Exits loop if no more implication is applied
SmallSetVector<StringRef, 16> WorkList;
Expand Down