Skip to content

Commit

Permalink
[RISCV] Remove SeenExtMap from RISCVISAInfo::parseArchString. (llvm#9…
Browse files Browse the repository at this point in the history
…7506)

Use the Exts map directly instead of adding to a temporary MapVector
first.

There are a couple functional change from this.
-If an unknown extension is duplicated, we will now print an error for
it being unknown instead of an error for it being duplicated. 
-If an unknown extension is followed by an underscore with no extension after
it, we will error for the unknown extension instead of the dangling
underscore.

These don't seem like serious changes to me. I've updated tests
accordingly.
  • Loading branch information
topperc authored and kbluck committed Jul 6, 2024
1 parent 0d67764 commit b6df62d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 25 deletions.
8 changes: 4 additions & 4 deletions clang/test/Driver/riscv-arch.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_ -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XSEP %s
// RV32-XSEP: error: invalid arch name 'rv32ixabc_',
// RV32-XSEP: extension name missing after separator '_'
// RV32-XSEP: unsupported non-standard user-level extension 'xabc'

// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_a -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-PREFIX %s
Expand All @@ -318,10 +318,10 @@
// RV32-X-ORDER: error: invalid arch name 'rv32ixdef_sabc',
// RV32-X-ORDER unsupported non-standard user-level extension 'xdef'

// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_xabc -### %s \
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32im_m -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XDUP %s
// RV32-XDUP: error: invalid arch name 'rv32ixabc_xabc',
// RV32-XDUP: duplicated non-standard user-level extension 'xabc'
// RV32-XDUP: error: invalid arch name 'rv32im_m',
// RV32-XDUP: duplicated standard user-level extension 'm'

// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_xdef -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-X-INVAL %s
Expand Down
32 changes: 11 additions & 21 deletions llvm/lib/TargetParser/RISCVISAInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
//===----------------------------------------------------------------------===//

#include "llvm/TargetParser/RISCVISAInfo.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
Expand Down Expand Up @@ -558,9 +556,6 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
"profile name");

std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
MapVector<std::string, RISCVISAUtils::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 All @@ -583,8 +578,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
EnableExperimentalExtension, ExperimentalExtensionVersionCheck))
return std::move(E);

// Postpone AddExtension until end of this function
SeenExtMap[StringRef(&Baseline, 1).str()] = {Major, Minor};
ISAInfo->Exts[std::string(1, Baseline)] = {Major, Minor};
break;
case 'g':
// g expands to extensions in RISCVGImplications.
Expand All @@ -597,11 +591,11 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// No matter which version is given to `g`, we always set imafd to default
// version since the we don't have clear version scheme for that on
// ISA spec.
for (const auto *Ext : RISCVGImplications) {
for (const char *Ext : RISCVGImplications) {
auto Version = findDefaultVersion(Ext);
assert(Version && "Default extension version not found?");
// Postpone AddExtension until end of this function
SeenExtMap[Ext] = {Version->Major, Version->Minor};
ISAInfo->Exts[std::string(Ext)] = {Version->Major, Version->Minor};
}
break;
}
Expand Down Expand Up @@ -662,23 +656,19 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
if (Name.size() == 1)
Ext = Ext.substr(ConsumeLength);

// Check if duplicated extension.
if (SeenExtMap.contains(Name.str()))
if (!RISCVISAInfo::isSupportedExtension(Name))
return getErrorForInvalidExt(Name);

// Insert and error for duplicates.
if (!ISAInfo->Exts
.emplace(Name.str(),
RISCVISAUtils::ExtensionVersion{Major, Minor})
.second)
return getError("duplicated " + Desc + " '" + Name + "'");

SeenExtMap[Name.str()] = {Major, Minor};
} while (!Ext.empty());
}

// Check all Extensions are supported.
for (auto &SeenExtAndVers : SeenExtMap) {
const std::string &ExtName = SeenExtAndVers.first;

if (!RISCVISAInfo::isSupportedExtension(ExtName))
return getErrorForInvalidExt(ExtName);
ISAInfo->Exts[ExtName] = SeenExtAndVers.second;
}

return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
}

Expand Down

0 comments on commit b6df62d

Please sign in to comment.