Skip to content

Commit

Permalink
Revert "[RISCV] Relax march string order constraint" (#79976)
Browse files Browse the repository at this point in the history 
Reverts #78120

Buildbot is broken:

llvm/lib/Support/RISCVISAInfo.cpp:910:18: error: call to deleted
constructor of 'llvm::Error'
          return E;
                 ^
  • Loading branch information
@joker-eph
joker-eph committed Jan 30, 2024
1 parent c61686e commit 5a00cb1
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 246 deletions.
14 changes: 8 additions & 6 deletions clang/test/Driver/riscv-arch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,9 @@
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s
// RV32L: error: invalid arch name 'rv32l'

// RUN: %clang --target=riscv32-unknown-elf -march=rv32imadf -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck %s
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32imadf -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMADF %s
// RV32IMADF: error: invalid arch name 'rv32imadf'

// RUN: not %clang --target=riscv32-unknown-elf -march=rv32imm -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMM %s
Expand All @@ -183,8 +184,9 @@
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64L %s
// RV64L: error: invalid arch name 'rv64l'

// RUN: %clang --target=riscv64-unknown-elf -march=rv64imadf -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck %s
// RUN: not %clang --target=riscv64-unknown-elf -march=rv64imadf -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMADF %s
// RV64IMADF: error: invalid arch name 'rv64imadf'

// RUN: not %clang --target=riscv64-unknown-elf -march=rv64imm -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMM %s
Expand Down Expand Up @@ -214,7 +216,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32imcq -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ORDER %s
// RV32-ORDER: error: invalid arch name 'rv32imcq',
// RV32-ORDER: unsupported standard user-level extension 'q'
// RV32-ORDER: standard user-level extension not given in canonical order 'q'

// RUN: not %clang --target=riscv32-unknown-elf -march=rv32izvl64b -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZVL64B-ER %s
Expand Down Expand Up @@ -316,7 +318,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_a -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-PREFIX %s
// RV32-PREFIX: error: invalid arch name 'rv32ixabc_a',
// RV32-PREFIX: unsupported non-standard user-level extension 'xabc'
// RV32-PREFIX: invalid extension prefix 'a'

// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixdef_sabc -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-ORDER %s
Expand Down
302 changes: 145 additions & 157 deletions llvm/lib/Support/RISCVISAInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//

#include "llvm/Support/RISCVISAInfo.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/StringExtras.h"
Expand Down Expand Up @@ -704,106 +703,6 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
return std::move(ISAInfo);
}

static Error splitExtsByUnderscore(StringRef Exts,
std::vector<std::string> &SplitExts) {
SmallVector<StringRef, 8> Split;
if (Exts.empty())
return Error::success();

Exts.split(Split, "_");

for (auto Ext : Split) {
if (Ext.empty())
return createStringError(errc::invalid_argument,
"extension name missing after separator '_'");

SplitExts.push_back(Ext.str());
}
return Error::success();
}

static Error processMultiLetterExtension(
StringRef RawExt,
MapVector<std::string, RISCVISAInfo::ExtensionVersion,
std::map<std::string, unsigned>> &SeenExtMap,
bool IgnoreUnknown, bool EnableExperimentalExtension,
bool ExperimentalExtensionVersionCheck) {
StringRef Type = getExtensionType(RawExt);
StringRef Desc = getExtensionTypeDesc(RawExt);
auto Pos = findLastNonVersionCharacter(RawExt) + 1;
StringRef Name(RawExt.substr(0, Pos));
StringRef Vers(RawExt.substr(Pos));

if (Type.empty()) {
if (IgnoreUnknown)
return Error::success();
return createStringError(errc::invalid_argument,
"invalid extension prefix '" + RawExt + "'");
}

if (!IgnoreUnknown && Name.size() == Type.size())
return createStringError(errc::invalid_argument,
"%s name missing after '%s'", Desc.str().c_str(),
Type.str().c_str());

unsigned Major, Minor, ConsumeLength;
if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength,
EnableExperimentalExtension,
ExperimentalExtensionVersionCheck)) {
if (IgnoreUnknown) {
consumeError(std::move(E));
return Error::success();
}
return E;
}

// Check if duplicated extension.
if (!IgnoreUnknown && SeenExtMap.contains(Name.str()))
return createStringError(errc::invalid_argument, "duplicated %s '%s'",
Desc.str().c_str(), Name.str().c_str());

if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
return Error::success();

SeenExtMap[Name.str()] = {Major, Minor};
return Error::success();
}

static Error processSingleLetterExtension(
StringRef &RawExt,
MapVector<std::string, RISCVISAInfo::ExtensionVersion,
std::map<std::string, unsigned>> &SeenExtMap,
bool IgnoreUnknown, bool EnableExperimentalExtension,
bool ExperimentalExtensionVersionCheck) {
unsigned Major, Minor, ConsumeLength;
StringRef Name = RawExt.take_front(1);
RawExt.consume_front(Name);
if (auto E = getExtensionVersion(Name, RawExt, Major, Minor, ConsumeLength,
EnableExperimentalExtension,
ExperimentalExtensionVersionCheck)) {
if (IgnoreUnknown) {
consumeError(std::move(E));
RawExt = RawExt.substr(ConsumeLength);
return Error::success();
}
return E;
}

RawExt = RawExt.substr(ConsumeLength);

// Check if duplicated extension.
if (!IgnoreUnknown && SeenExtMap.contains(Name.str()))
return createStringError(errc::invalid_argument,
"duplicated standard user-level extension '%s'",
Name.str().c_str());

if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
return Error::success();

SeenExtMap[Name.str()] = {Major, Minor};
return Error::success();
}

llvm::Expected<std::unique_ptr<RISCVISAInfo>>
RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
bool ExperimentalExtensionVersionCheck,
Expand All @@ -824,9 +723,6 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,

unsigned XLen = HasRV64 ? 64 : 32;
std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
MapVector<std::string, RISCVISAInfo::ExtensionVersion,
std::map<std::string, unsigned>>
SeenExtMap;

// The canonical order specified in ISA manual.
// Ref: Table 22.1 in RISC-V User-Level ISA V2.2
Expand Down Expand Up @@ -857,6 +753,17 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// Skip rvxxx
StringRef Exts = Arch.substr(5);

// Remove multi-letter standard extensions, non-standard extensions and
// supervisor-level extensions. They have 'z', 'x', 's' prefixes.
// Parse them at the end.
// Find the very first occurrence of 's', 'x' or 'z'.
StringRef OtherExts;
size_t Pos = Exts.find_first_of("zsx");
if (Pos != StringRef::npos) {
OtherExts = Exts.substr(Pos);
Exts = Exts.substr(0, Pos);
}

unsigned Major, Minor, ConsumeLength;
if (Baseline == 'g') {
// Versions for g are disallowed, and this was checked for previously.
Expand All @@ -866,10 +773,9 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// version since the we don't have clear version scheme for that on
// ISA spec.
for (const auto *Ext : RISCVGImplications) {
if (auto Version = findDefaultVersion(Ext)) {
// Postpone AddExtension until end of this function
SeenExtMap[Ext] = {Version->Major, Version->Minor};
} else
if (auto Version = findDefaultVersion(Ext))
ISAInfo->addExtension(Ext, *Version);
else
llvm_unreachable("Default extension version not found?");
}
} else {
Expand All @@ -887,69 +793,151 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
Minor = Version->Minor;
}

// Postpone AddExtension until end of this function
SeenExtMap[StringRef(&Baseline, 1).str()] = {Major, Minor};
ISAInfo->addExtension(StringRef(&Baseline, 1), {Major, Minor});
}

// Consume the base ISA version number and any '_' between rvxxx and the
// first extension
Exts = Exts.drop_front(ConsumeLength);
Exts.consume_front("_");

std::vector<std::string> SplittedExts;
if (auto E = splitExtsByUnderscore(Exts, SplittedExts))
return std::move(E);

for (auto &Ext : SplittedExts) {
StringRef CurrExt = Ext;
while (!CurrExt.empty()) {
if (AllStdExts.contains(CurrExt.front())) {
if (auto E = processSingleLetterExtension(
CurrExt, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
ExperimentalExtensionVersionCheck))
return E;
} else if (CurrExt.front() == 'z' || CurrExt.front() == 's' ||
CurrExt.front() == 'x') {
// Handle other types of extensions other than the standard
// general purpose and standard user-level extensions.
// Parse the ISA string containing non-standard user-level
// extensions, standard supervisor-level extensions and
// non-standard supervisor-level extensions.
// These extensions start with 'z', 's', 'x' prefixes, might have a
// version number (major, minor) and are separated by a single
// underscore '_'. We do not enforce a canonical order for them.
if (auto E = processMultiLetterExtension(
CurrExt, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
ExperimentalExtensionVersionCheck))
return E;
// Multi-letter extension must be seperate following extension with
// underscore
break;
} else {
// FIXME: Could it be ignored by IgnoreUnknown?
return createStringError(errc::invalid_argument,
"invalid standard user-level extension '%c'",
CurrExt.front());
auto StdExtsItr = StdExts.begin();
auto StdExtsEnd = StdExts.end();
auto GoToNextExt = [](StringRef::iterator &I, unsigned ConsumeLength,
StringRef::iterator E) {
I += 1 + ConsumeLength;
if (I != E && *I == '_')
++I;
};
for (auto I = Exts.begin(), E = Exts.end(); I != E;) {
char C = *I;

// Check ISA extensions are specified in the canonical order.
while (StdExtsItr != StdExtsEnd && *StdExtsItr != C)
++StdExtsItr;

if (StdExtsItr == StdExtsEnd) {
// Either c contains a valid extension but it was not given in
// canonical order or it is an invalid extension.
if (StdExts.contains(C)) {
return createStringError(
errc::invalid_argument,
"standard user-level extension not given in canonical order '" +
Twine(C) + "'");
}

return createStringError(errc::invalid_argument,
"invalid standard user-level extension '" +
Twine(C) + "'");
}

// Move to next char to prevent repeated letter.
++StdExtsItr;

StringRef Next;
unsigned Major, Minor, ConsumeLength;
if (std::next(I) != E)
Next = StringRef(std::next(I), E - std::next(I));
if (auto E = getExtensionVersion(StringRef(&C, 1), Next, Major, Minor,
ConsumeLength, EnableExperimentalExtension,
ExperimentalExtensionVersionCheck)) {
if (IgnoreUnknown) {
consumeError(std::move(E));
GoToNextExt(I, ConsumeLength, Exts.end());
continue;
}
return std::move(E);
}

// The order is OK, then push it into features.
// Currently LLVM supports only "mafdcvh".
if (!isSupportedExtension(StringRef(&C, 1))) {
if (IgnoreUnknown) {
GoToNextExt(I, ConsumeLength, Exts.end());
continue;
}
return createStringError(errc::invalid_argument,
"unsupported standard user-level extension '" +
Twine(C) + "'");
}
ISAInfo->addExtension(StringRef(&C, 1), {Major, Minor});

// Consume full extension name and version, including any optional '_'
// between this extension and the next
GoToNextExt(I, ConsumeLength, Exts.end());
}

// Check all Extensions are supported.
for (auto &SeenExtAndVers : SeenExtMap) {
const std::string &ExtName = SeenExtAndVers.first;
RISCVISAInfo::ExtensionVersion ExtVers = SeenExtAndVers.second;
// Handle other types of extensions other than the standard
// general purpose and standard user-level extensions.
// Parse the ISA string containing non-standard user-level
// extensions, standard supervisor-level extensions and
// non-standard supervisor-level extensions.
// These extensions start with 'z', 's', 'x' prefixes, might have a version
// number (major, minor) and are separated by a single underscore '_'. We do
// not enforce a canonical order for them.
// Set the hardware features for the extensions that are supported.

// Multi-letter extensions are seperated by a single underscore
// as described in RISC-V User-Level ISA V2.2.
SmallVector<StringRef, 8> Split;
OtherExts.split(Split, '_');

if (!RISCVISAInfo::isSupportedExtension(ExtName)) {
if (ExtName.size() == 1) {
return createStringError(
errc::invalid_argument,
"unsupported standard user-level extension '%s'", ExtName.c_str());
SmallVector<StringRef, 8> AllExts;
if (Split.size() > 1 || Split[0] != "") {
for (StringRef Ext : Split) {
if (Ext.empty())
return createStringError(errc::invalid_argument,
"extension name missing after separator '_'");

StringRef Type = getExtensionType(Ext);
StringRef Desc = getExtensionTypeDesc(Ext);
auto Pos = findLastNonVersionCharacter(Ext) + 1;
StringRef Name(Ext.substr(0, Pos));
StringRef Vers(Ext.substr(Pos));

if (Type.empty()) {
if (IgnoreUnknown)
continue;
return createStringError(errc::invalid_argument,
"invalid extension prefix '" + Ext + "'");
}

if (!IgnoreUnknown && Name.size() == Type.size())
return createStringError(errc::invalid_argument,
Desc + " name missing after '" + Type + "'");

unsigned Major, Minor, ConsumeLength;
if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength,
EnableExperimentalExtension,
ExperimentalExtensionVersionCheck)) {
if (IgnoreUnknown) {
consumeError(std::move(E));
continue;
}
return std::move(E);
}
return createStringError(errc::invalid_argument, "unsupported %s '%s'",
getExtensionTypeDesc(ExtName).str().c_str(),
ExtName.c_str());

// Check if duplicated extension.
if (!IgnoreUnknown && llvm::is_contained(AllExts, Name))
return createStringError(errc::invalid_argument,
"duplicated " + Desc + " '" + Name + "'");

if (IgnoreUnknown && !isSupportedExtension(Name))
continue;

ISAInfo->addExtension(Name, {Major, Minor});
// Extension format is correct, keep parsing the extensions.
// TODO: Save Type, Name, Major, Minor to avoid parsing them later.
AllExts.push_back(Name);
}
}

for (auto Ext : AllExts) {
if (!isSupportedExtension(Ext)) {
StringRef Desc = getExtensionTypeDesc(getExtensionType(Ext));
return createStringError(errc::invalid_argument,
"unsupported " + Desc + " '" + Ext + "'");
}
ISAInfo->addExtension(ExtName, ExtVers);
}

return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
Expand Down

0 comments on commit 5a00cb1

Please sign in to comment.