[RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[kito-cheng]

Summary

How many place you need to modify when implementing a new extension for RISC-V?

At least 7 places as I know:

• Add new SubtargetFeature at RISCV.td
• -march parser in RISCV.cpp
• RISCVTargetInfo::[e-mail] for handling feature vector.
• RISCVTargetInfo::[e-mail] for pre-define marco.
• Arch string parser for ELF attribute in RISCVAsmParser.cpp
• ELF attribute emittion in RISCVAsmParser.cpp, and make sure it's in canonical order...
• ELF attribute emittion in RISCVTargetStreamer.cpp, and again, must in canonical order...

And now, this patch provide an unified infrastructure for handling (almost)
everything of RISC-V arch string.

After this patch, you only need to update 2 places for implement an extension
for RISC-V:

• Add new SubtargetFeature at RISCV.td, hmmm, it's hard to avoid.
• Add new entry to [e-mail].

Most codes are come from existing -march parser, but with few new feature/bug
fixes:

• Accept version for -march, e.g. -march=rv32i2p0.
• Reject version info with p but without minor version number like rv32i2p.

[kito-cheng]

Changes:

• Using RISCVISAInfo in riscv::getRISCVABI
• Allow rv32e for -march.
• Address review comments.

[kito-cheng]

llvm/include/llvm/Support/RISCVISAInfo.h

43

All extension are added in parse function, but there might require a lookup and add if not found loop when implementing (more complete*) implied extension rule in future, so using ordered container would make this easier.

• We don't implement full implied extension rule, like f implied zicsr, d implied f...

[khchen]

clang/test/Driver/riscv-abi.c

68 ↗

(On Diff #358130)

Why do we need to modify here?

[kito-cheng]

Changes:

• Handle arch attribute emition in RISCVTargetStreamer.cpp, thanks @khchen for reminding me that!

[khchen]

This all looks good to me except some tidy warning.
wait for others comments.

llvm/lib/Support/RISCVISAInfo.cpp

461

nit: add break; to avoid the implicit-fallthrough warning.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

57

nit: please fix this tidy warning

[kito-cheng]

llvm/lib/Support/RISCVISAInfo.cpp

461

Oh, thanks, it seems like more than nit :p

[kito-cheng]

Changes:

• Address @khchen's comment.

[craig.topper]

llvm/lib/Support/RISCVISAInfo.cpp

41

Make this static constexpr StringLiteral AllStdExts = "mafdqlcbjtpvn";

397

This comment should be "Drop '+' or '-'" I think?

545

This is an unusual placement of curly braces. Put the comment inside?

[jrtc27]

clang/test/Driver/riscv-abi.c

68 ↗

(On Diff #358130)

rv64d is not a valid arch string. rv64id is though; would that work here? These tests are just currently testing behaviour that _should_ have been an error... (see also rv64 f above)

clang/test/Driver/riscv-arch.c

198–201

Hm, given we don't currently support RV32E properly this should probably be an error still, but could be a "nicer" error than a generic "invalid arch name" (which is technically wrong)

llvm/include/llvm/Support/RISCVISAInfo.h

46

These comments aren't helpful. If you want to write full doxygen then you can, but a one-line comment that won't appear in documentation and is obvious from the function name and signature is just clutter.

llvm/lib/Support/RISCVISAInfo.cpp

25

This seems unnecessary

33

Ditto for these

41

I can't help but feel this is really an array of chars, not a string. We don't even need the trailing NUL, though double quote syntax is simpler than curly braces and a bunch of single-quote chars just to save a byte.

44

but that's the default so I'd omit it for anything other than the cases where it's true

72–73

Comment isn't useful, it's a trivial wrapper

78

Can this not go in the header as a trivial constructor?

113

Obvious comment

136

*Extensions* don't have an experimental- prefix, *internal target features* do, so something strange is going on here. This feels like we're confusing several things:

1. Whether or not Ext is a feature or an extension
2. Whether or not Ext is experimental
3. Whether we want to look at experimental extensions

Some of those are somewhat necessarily interwoven, but the naming does not currently accurately reflect what these things mean, and I would argue we should be very explicit and keep features and extensions separate, never using the same thing to represent both.

194

This is wrong as it doesn't include the +2 to account for I and E, though does happen to work because A and B are both known single-letter standard extensions so you can't actually get it to overlap (but were AllStdExts to omit one of them you would be able to).

IMO though it would be cleaner to use negative numbers for I and E, as they're special, and that means you can just start the counting at 0 for all the others, not needing the +2 anywhere.

236–237

This belongs as a more fully-fledged doxgen comment in the header.

454

Yet you removed the error for it?

549

596–597

(wrapped appropriately)

[kito-cheng]

Changes:

• New function RISCVISAInfo::isSupportedExtensionFeature
• Remove parameter CheckExperimental from RISCVISAInfo::isSupportedExtension
• Clean up obvious comments
• Address @jrtc27's and @craig.topper's comments

[kito-cheng]

clang/test/Driver/riscv-arch.c

198–201

We support E-extension on MC-layer...but not support ilp32e.

This patch unify the ISA stuffs, so either I need to remove that from MC, or I need to made rv32e supported on driver...

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp#L50
https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/RISCV/rv32e-valid.s
https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/RISCV/rv32e-invalid.s

llvm/lib/Support/RISCVISAInfo.cpp

41

Yeah, it's actually just an array of chars, but we have use find function from StringRef :p

136

Good point, CheckExperimental is kind of ambiguous, new function: isSupportedExtensionFeature added.

[kito-cheng]

Changes:

• Rename RISCVISAInfo::parse to RISCVISAInfo::parseFeatures/RISCVISAInfo::parseArchString

[kito-cheng]

llvm/include/llvm/Support/RISCVISAInfo.h

46

Rename RISCVISAInfo::parse to RISCVISAInfo::parseFeatures/RISCVISAInfo::parseArchString, I guess it should be more clear.

[frasercrmck]

llvm/lib/Support/RISCVISAInfo.cpp

128

This looks like a find_if if that'd make it any simpler.

148

This also looks like a find_if

232

Not sure why these need to be declared up here.

311

Failed to parse ...

316

Failed to parse

360

Commented-out code

415

I think you can just have if (llvm::any_of(Arch, isupper)) here.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2039

Don't need {} here

2058

Don't need {} here either

[kito-cheng]

Changes:

• Address @frasercrmck's comment

[jrtc27]

llvm/include/llvm/Support/RISCVISAInfo.h

32

Does Exts need initialising to be empty here? I can never remember

48–53

I wonder, should these be constructors instead? RISCVISAInfo ISAInfo; ISAInfo.parse doesn't feel very idiomatic C++

llvm/lib/Support/RISCVISAInfo.cpp

41

Still not a StringLiteral

44

I do think it would be better to not list Experimental for the non-experimental ones

179

Why not just return these directly? Don't need a variable.

195

I imagine this needs to change to the new Sm-/Ss- scheme, but I don't know which way round those are intended to go

198

Do we know if this is still the case? Ss- is being used for S-mode extensions and Sm- for M-mode extensions, so I'd expect Sh- (or maybe just Ss-) for HS-mode extensions, not H-.

208

llvm_unreachable

215

Why not put this in its switch case?

226–230

Don't know if this is better or not, but this is the more compact way to write it

232

Yeah, declare the variables at their assignments. Besides, you don't even need variables for the single-letter case, just compare the return values of the functions directly.

[kito-cheng]

Changes:

• Forbid copy ctor and operator= for RISCVISAInfo.
• Move RISCVISAInfo's constructor to private.
• Refine RISCVISAInfo::parse* and made they become static function.
• Address @jrtc27's comment.

[kito-cheng]

llvm/include/llvm/Support/RISCVISAInfo.h

32

std::map has default construct that will construct an empty map.

llvm/lib/Support/RISCVISAInfo.cpp

195

I would prefer submit another patch to make this parser up to date, and keep this patch as a refactor patch as possible:

e.g.:

• Full implication info, e.g. d implied f, f implied zicsr...
• Prefix zxm.

198

Like above reply, prefer another patch to fix that.

226–230

It's more compact but it's hard to read the rule to me, so I would prefer keep that as it is

[jrtc27]

llvm/include/llvm/Support/RISCVISAInfo.h

50–51

Use llvm::Expected<...> as the return value to avoid a separate error out param

llvm/lib/Support/RISCVISAInfo.cpp

41

Is the "Infos" suffix on the variable name really needed?

42

I'd keep these all one per line

108

379

No error?..

[kito-cheng]

llvm/lib/Support/RISCVISAInfo.cpp

42

Although it's format by clang-format, but I like one per line too, let me manually update that.

108

It's format by clang-format, but either is fine to me :P

379

Not all features is related to extension, like relax or save-restore, so here we just ignore, but add few comment here seem better.

[kito-cheng]

Changes:

• Rename class, strip the Info:
  • RISCVSupportedExtensionInfo -> RISCVSupportedExtension
• Rename variables:
  • SupportedExtensionInfos -> SupportedExtensions
  • SupportedExperimentalExtensionInfos -> SupportedExperimentalExtensions
• Change return type of parseArchString to llvm::Expected<std::unique_ptr<RISCVISAInfo>>
• Address @jrtc27's comment.

[kito-cheng]

ping

[asb]

I'm getting a build error (building with clang 12.0.1):

FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/RISCVISAInfo.cpp.o 
/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/asb/llvm-project/build/default/lib/Support -I/home/asb/llvm-project/llvm/lib/Support -I/home/asb/llvm-project/build/default/include -I/home/asb/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -Werror=global-constructors -g -fPIC -gsplit-dwarf -std=c++14  -fno-exceptions -fno-rtti -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/RISCVISAInfo.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/RISCVISAInfo.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/RISCVISAInfo.cpp.o -c /home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp
/home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp:399:10: warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
  return std::move(ISAInfo);
         ^
/home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp:399:10: note: remove std::move call here
  return std::move(ISAInfo);
         ^~~~~~~~~~       ~
/home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp:478:12: error: call to deleted constructor of 'llvm::Error'
    return E;
           ^
/home/asb/llvm-project/llvm/include/llvm/Support/Error.h:186:3: note: 'Error' has been explicitly marked deleted here
  Error(const Error &Other) = delete;
  ^
/home/asb/llvm-project/llvm/include/llvm/Support/Error.h:493:18: note: passing argument to parameter 'Err' here
  Expected(Error Err)
                 ^
/home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp:520:14: error: call to deleted constructor of 'llvm::Error'
      return E;
             ^
/home/asb/llvm-project/llvm/include/llvm/Support/Error.h:186:3: note: 'Error' has been explicitly marked deleted here
  Error(const Error &Other) = delete;
  ^
/home/asb/llvm-project/llvm/include/llvm/Support/Error.h:493:18: note: passing argument to parameter 'Err' here
  Expected(Error Err)
                 ^
/home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp:642:14: error: call to deleted constructor of 'llvm::Error'
      return E;
             ^
/home/asb/llvm-project/llvm/include/llvm/Support/Error.h:186:3: note: 'Error' has been explicitly marked deleted here
  Error(const Error &Other) = delete;
  ^
/home/asb/llvm-project/llvm/include/llvm/Support/Error.h:493:18: note: passing argument to parameter 'Err' here
  Expected(Error Err)
                 ^
1 warning and 3 errors generated.
[3/3711] Building CXX object tools/llvm-config/CMakeFiles/llvm-config.dir/llvm-config.cpp.o
ninja: build stopped: subcommand failed.

[kito-cheng]

Changes:

• Fix build warning and build error with clang

[kito-cheng]

@asb Thanks for report that, there is no warning and build error when I build with GCC 7 (which is default compiler in Ubuntu 18.04), but I can reproduce that with clang 13, seems like I should switch my default compiler :P

[eopXD]

llvm/lib/Support/RISCVISAInfo.cpp

269

For naming consistency with parseArchString, maybe rename MArch to Arch?

[kito-cheng]

Changes:

• Remove unused argument MArch for getExtensionVersion.

[kito-cheng]

llvm/lib/Support/RISCVISAInfo.cpp

269

It turns out the argument isn't used anywhere...so I remove that.

[Jim]

llvm/lib/Support/RISCVISAInfo.cpp

388

Does this need to check it is invalid if XLen is 64?

395

Why not get the version of i from SupportedExtensions?

[kito-cheng]

Changes:

• Check feature combination is valid.
• Change return type of parseFeatures to llvm::Expected<std::unique_ptr<RISCVISAInfo>>.
• Set default extension version in getExtensionVersion.

[kito-cheng]

llvm/lib/Support/RISCVISAInfo.cpp

388

Add check, and parseFeatures might return Error now.

395

Updated :)

[Jim]

llvm/lib/Support/RISCVISAInfo.cpp

417

Could this checking put before ISAInfo->addExtension...

[kito-cheng]

Changes:

• Address Jim's comment.

[luismarques]

I was trying to put this patch through its paces but it no longer applies. Can you please rebase it? It seems this is nearly there.

[kito-cheng]

Seems like conflict with D108187, will update after testing :)

[kito-cheng]

Changes:

• Rebase to main.

[kito-cheng]

ping

[kito-cheng]

Changes:

• Rebase to main
• Remove b and zbproposedc.

[craig.topper]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

209

Please add an inline comment for what "true" represents.

210

Should we have an llvm_unreachable here for unknown values of XLen?

221

Drop else after return

223

here too

228

Here too

llvm/include/llvm/Support/RISCVISAInfo.h

44

I'm not sure the comparison to StringMap is helpful here. StringMap is an unordered map that stores strings in the same allocation as the value. This std::map is ordered and doesn't store the strings in the same allocation.

llvm/lib/Support/RISCVISAInfo.cpp

287

Do MajorStr and MinorStr need to be std::strings or can they be StringRefs? Looks like they are either empty or a subset of In. So there shouldn't be a lifetime issue?

353

Should this message say which extension the error applies to?

389

Use

auto ISAInfo = std::make_unique<RISCVISAInfo>()

391

Can we make XLen part of the RISCVISAInfo constructor? Seems like we know that early enough.

441

Can we delay constructing this until after the first 2 error checks? Then we could pass XLen to the constructor.

710

Would this better as

Arch << "rv" << XLen;

That makes it future proof to a hypothetical rv128 someday.

[kito-cheng]

Changes:

• Address @craig.topper's comment
• Add XLen to constructor of RISCVISAInfo.

llvm/lib/Support/RISCVISAInfo.cpp

389

std::make_unique<RISCVISAInfo>() require a public constructor, but I would prefer keep that in private.

[craig.topper]

LGTM other than that comment about removing 'b' from AllStdExts.

llvm/lib/Support/RISCVISAInfo.cpp

40

'b' shouldn't be in this list anymore?

389

I agree with keeping the constructor private. I didn't realize make_unique had that restriction.

[khchen]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2070–2071

maybe we could have a NFC patch later to add

// Emit the arch string if needed.
if (!IsIntegerValue)
  getTargetStreamer().emitTextAttribute(Tag, ISAInfo->toString());

here to reuse ISAInfo, and remove 2098~2117 code.

[kito-cheng]

llvm/lib/Support/RISCVISAInfo.cpp

40

I would prefer to keep that for sync with ISA manual, like other unsupported extension like J, T and `N, we remove B from SupportedExtensions, so keep this should be harmless.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2070–2071

Good point, thanks :)
Let improve that by following patch.

[kito-cheng]

Committed with one minor update for version of zba/zbb/zbc/zbs.

[kito-cheng]

llvm/lib/Support/RISCVISAInfo.cpp

40

Oh, and it also used for compute the canonical order for z* extensions.

[akuegel]

llvm/include/llvm/Support/RISCVISAInfo.h

16

Would it be possible to avoid the usage of ArgList in a target that belongs to Support?
This introduces a dependency cycle, since Option already depends on Support.

[teemperor]

This broke the modules build:

While building module 'LLVM_Utils' imported from lvm/lib/Support/TargetParser.cpp:14:
In file included from <module-includes>:209:
llvm/include/llvm/Support/RISCVISAInfo.h:15:10: fatal error: could not build module 'LLVM_Option'
#include "llvm/Option/ArgList.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
llvm/lib/Support/TargetParser.cpp:14:10: fatal error: could not build module 'LLVM_Utils'
#include "llvm/Support/TargetParser.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Support headers can't include Option headers (as Option depends on Support already, so that would be a cyclic dependency). I believe folks here are in the US time zone, so I went ahead and replaced the header include with a forward decl to unbreak the bots (see de4d2f80b75e2a1e4b0ac5c25e20f20839633688 )

FWIW, I think there is a better place than Support for this new class. Support is a dependency of nearly every single LLVM component, but this class seems to be used by a handful of files. Could we maybe move this to some other/new part of LLVM (and make the few components that need it depend on that)?

[craig.topper]

In D105168#3069499, @teemperor wrote:

This broke the modules build:

While building module 'LLVM_Utils' imported from lvm/lib/Support/TargetParser.cpp:14:
In file included from <module-includes>:209:
llvm/include/llvm/Support/RISCVISAInfo.h:15:10: fatal error: could not build module 'LLVM_Option'
#include "llvm/Option/ArgList.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
llvm/lib/Support/TargetParser.cpp:14:10: fatal error: could not build module 'LLVM_Utils'
#include "llvm/Support/TargetParser.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Support headers can't include Option headers (as Option depends on Support already, so that would be a cyclic dependency). I believe folks here are in the US time zone, so I went ahead and replaced the header include with a forward decl to unbreak the bots (see de4d2f80b75e2a1e4b0ac5c25e20f20839633688 )

FWIW, I think there is a better place than Support for this new class. Support is a dependency of nearly every single LLVM component, but this class seems to be used by a handful of files. Could we maybe move this to some other/new part of LLVM (and make the few components that need it depend on that)?

Thanks for the header fix. I think we also need to fix the library circular dependency that still exists. The Support library should not use anything from the Option library. Maybe we can partition the function some way that moves the MakeArgStr back into the clang driver code.

I don't know if we have a better library for this new class. I think Support is the only common library between the clang driver and LLVM's MC layer. I supposed we could create a new library, but that might be a hard sell for one file.

[teemperor]

In D105168#3071152, @craig.topper wrote:

In D105168#3069499, @teemperor wrote:

This broke the modules build:

While building module 'LLVM_Utils' imported from lvm/lib/Support/TargetParser.cpp:14:
In file included from <module-includes>:209:
llvm/include/llvm/Support/RISCVISAInfo.h:15:10: fatal error: could not build module 'LLVM_Option'
#include "llvm/Option/ArgList.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
llvm/lib/Support/TargetParser.cpp:14:10: fatal error: could not build module 'LLVM_Utils'
#include "llvm/Support/TargetParser.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Support headers can't include Option headers (as Option depends on Support already, so that would be a cyclic dependency). I believe folks here are in the US time zone, so I went ahead and replaced the header include with a forward decl to unbreak the bots (see de4d2f80b75e2a1e4b0ac5c25e20f20839633688 )

FWIW, I think there is a better place than Support for this new class. Support is a dependency of nearly every single LLVM component, but this class seems to be used by a handful of files. Could we maybe move this to some other/new part of LLVM (and make the few components that need it depend on that)?

Thanks for the header fix. I think we also need to fix the library circular dependency that still exists. The Support library should not use anything from the Option library. Maybe we can partition the function some way that moves the MakeArgStr back into the clang driver code.

I don't know if we have a better library for this new class. I think Support is the only common library between the clang driver and LLVM's MC layer. I supposed we could create a new library, but that might be a hard sell for one file.

IIRC there are some other target-specific classes in Support for the same reason. Maybe we can make something such as TargetSupport or SharedTarget?

[jrtc27]

Two options come to mind if we really need to be outputting a StringRef list:

1. (the simpler option) Pass in a Twine -> const char * lambda that the caller hooks up to Args.MakeArgString
2. (probably the nicer option) Invent our own MakeArgString that allocates from storage owned by RISCVISAInfo itself; lots of ways you can do that

Adding a new library just so we can use MakeArgString seems overkill, even if there are other things that perhaps belong in something a bit less shared than Support.

[craig.topper]

In D105168#3071249, @jrtc27 wrote:

Two options come to mind if we really need to be outputting a StringRef list:

1. (the simpler option) Pass in a Twine -> const char * lambda that the caller hooks up to Args.MakeArgString

Good idea. I'll post a patch for that shortly.

2. (probably the nicer option) Invent our own MakeArgString that allocates from storage owned by RISCVISAInfo itself; lots of ways you can do that

I don't think the RISCVISAInfo objects lives long enough for that. It only lives for the duration of the function that calls toFeatures, but the Features vector is returned from that function.

Adding a new library just so we can use MakeArgString seems overkill, even if there are other things that perhaps belong in something a bit less shared than Support.

[jrtc27]

In D105168#3071321, @craig.topper wrote:

In D105168#3071249, @jrtc27 wrote:

Two options come to mind if we really need to be outputting a StringRef list:

1. (the simpler option) Pass in a Twine -> const char * lambda that the caller hooks up to Args.MakeArgString

Good idea. I'll post a patch for that shortly.

2. (probably the nicer option) Invent our own MakeArgString that allocates from storage owned by RISCVISAInfo itself; lots of ways you can do that

I don't think the RISCVISAInfo objects lives long enough for that. It only lives for the duration of the function that calls toFeatures, but the Features vector is returned from that function.

Hm, I see. We could also just have a global cache of heap-allocated copies of these strings, I guess; after all it's not all that different from the tables of strings we already have, just lazily built up at run time (can even store them in a lazily-initialised field of each RISCVExtensionInfo). Or we just stick to the slightly-ugly lambda approach, unless anyone else has bright ideas for a cleaner interface and implementation.

[craig.topper]

In D105168#3071330, @jrtc27 wrote:

In D105168#3071321, @craig.topper wrote:

In D105168#3071249, @jrtc27 wrote:

Two options come to mind if we really need to be outputting a StringRef list:

1. (the simpler option) Pass in a Twine -> const char * lambda that the caller hooks up to Args.MakeArgString

Good idea. I'll post a patch for that shortly.

2. (probably the nicer option) Invent our own MakeArgString that allocates from storage owned by RISCVISAInfo itself; lots of ways you can do that

I don't think the RISCVISAInfo objects lives long enough for that. It only lives for the duration of the function that calls toFeatures, but the Features vector is returned from that function.

Hm, I see. We could also just have a global cache of heap-allocated copies of these strings, I guess; after all it's not all that different from the tables of strings we already have, just lazily built up at run time (can even store them in a lazily-initialised field of each RISCVExtensionInfo). Or we just stick to the slightly-ugly lambda approach, unless anyone else has bright ideas for a cleaner interface and implementation.

Lambda approach here https://reviews.llvm.org/D112032

[zixuan-wu]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp

107 ↗

(On Diff #380228)

If SubtargetFeature is enum value like ARM.td does, Feature.Value would be a uninitialization value, so I think there should be if condition like Feature.Value < RISCV::NumSubtargetFeatures, or there is a out-of-range visit.

def ProcXXX :
  SubtargetFeature<"", "RISCVProcFamily",
                   "XXX", "XXX processors">;

[zixuan-wu]

Hi, @kito-cheng as your this patch unify the extension handing in one same place by new infra, are you going to support the multiple extension version in next step?

clang/lib/Basic/Targets/RISCV.h

 
 #ifndef LLVM_CLANG_LIB_BASIC_TARGETS_RISCV_H
 #define LLVM_CLANG_LIB_BASIC_TARGETS_RISCV_H
 
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/RISCVISAInfo.h"
 
 namespace clang {
 namespace targets {
 
 // RISC-V Target
 class RISCVTargetInfo : public TargetInfo {
 protected:
   std::string ABI, CPU;
-  bool HasM = false;
-  bool HasA = false;
-  bool HasF = false;
-  bool HasD = false;
-  bool HasC = false;
-  bool HasB = false;
-  bool HasV = false;
-  bool HasZba = false;
-  bool HasZbb = false;
-  bool HasZbc = false;
-  bool HasZbe = false;
-  bool HasZbf = false;
-  bool HasZbm = false;
-  bool HasZbp = false;
-  bool HasZbproposedc = false;
-  bool HasZbr = false;
-  bool HasZbs = false;
-  bool HasZbt = false;
-  bool HasZfh = false;
-  bool HasZvamo = false;
-  bool HasZvlsseg = false;
-
+  llvm::RISCVISAInfo ISAInfo;
   static const Builtin::Info BuiltinInfo[];
 
 public:
   RISCVTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
       : TargetInfo(Triple) {
     LongDoubleWidth = 128;
     LongDoubleAlign = 128;
     LongDoubleFormat = &llvm::APFloat::IEEEquad();
public:
   bool isValidCPUName(StringRef Name) const override;
   void fillValidCPUList(SmallVectorImpl<StringRef> &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
   void fillValidTuneCPUList(SmallVectorImpl<StringRef> &Values) const override;
 
   void setMaxAtomicWidth() override {
     MaxAtomicPromoteWidth = 128;
 
-    if (HasA)
+    if (ISAInfo.hasExtension("a"))
       MaxAtomicInlineWidth = 32;
   }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
 public:
   RISCV64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : RISCVTargetInfo(Triple, Opts) {
     LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
public:
   bool isValidCPUName(StringRef Name) const override;
   void fillValidCPUList(SmallVectorImpl<StringRef> &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
   void fillValidTuneCPUList(SmallVectorImpl<StringRef> &Values) const override;
 
   void setMaxAtomicWidth() override {
     MaxAtomicPromoteWidth = 128;
 
-    if (HasA)
+    if (ISAInfo.hasExtension("a"))
       MaxAtomicInlineWidth = 64;
   }
 };
 } // namespace targets
 } // namespace clang
 
 #endif // LLVM_CLANG_LIB_BASIC_TARGETS_RISCV_H

clang/lib/Basic/Targets/RISCV.cpp

 
 void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
                                        MacroBuilder &Builder) const {
   Builder.defineMacro("__ELF__");
   Builder.defineMacro("__riscv");
   bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64;
   Builder.defineMacro("__riscv_xlen", Is64Bit ? "64" : "32");
   StringRef CodeModel = getTargetOpts().CodeModel;
+  unsigned FLen = ISAInfo.getFLen();
   if (CodeModel == "default")
     CodeModel = "small";
 
   if (CodeModel == "small")
     Builder.defineMacro("__riscv_cmodel_medlow");
   else if (CodeModel == "medium")
     Builder.defineMacro("__riscv_cmodel_medany");
 
   StringRef ABIName = getABI();
   if (ABIName == "ilp32f" || ABIName == "lp64f")
     Builder.defineMacro("__riscv_float_abi_single");
   else if (ABIName == "ilp32d" || ABIName == "lp64d")
     Builder.defineMacro("__riscv_float_abi_double");
   else
     Builder.defineMacro("__riscv_float_abi_soft");
 
   if (ABIName == "ilp32e")
     Builder.defineMacro("__riscv_abi_rve");
 
   Builder.defineMacro("__riscv_arch_test");
-  Builder.defineMacro("__riscv_i", "2000000");
 
-  if (HasM) {
-    Builder.defineMacro("__riscv_m", "2000000");
+  for (auto &Extension : ISAInfo.getExtensions()) {
+    auto ExtName = Extension.first;
+    auto ExtInfo = Extension.second;
+    unsigned Version =
+        (ExtInfo.MajorVersion * 1000000) + (ExtInfo.MinorVersion * 1000);
+
+    Builder.defineMacro(Twine("__riscv_", ExtName), Twine(Version));
+  }
+
+  if (ISAInfo.hasExtension("m")) {
     Builder.defineMacro("__riscv_mul");
     Builder.defineMacro("__riscv_div");
     Builder.defineMacro("__riscv_muldiv");
   }
 
-  if (HasA) {
-    Builder.defineMacro("__riscv_a", "2000000");
+  if (ISAInfo.hasExtension("a")) {
     Builder.defineMacro("__riscv_atomic");
     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1");
     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2");
     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4");
     if (Is64Bit)
       Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
   }
 
-  if (HasF || HasD) {
-    Builder.defineMacro("__riscv_f", "2000000");
-    Builder.defineMacro("__riscv_flen", HasD ? "64" : "32");
+  if (FLen) {
+    Builder.defineMacro("__riscv_flen", Twine(FLen));
     Builder.defineMacro("__riscv_fdiv");
     Builder.defineMacro("__riscv_fsqrt");
   }
 
-  if (HasD)
-    Builder.defineMacro("__riscv_d", "2000000");
-
-  if (HasC) {
-    Builder.defineMacro("__riscv_c", "2000000");
+  if (ISAInfo.hasExtension("c"))
     Builder.defineMacro("__riscv_compressed");
-  }
 
-  if (HasB) {
-    Builder.defineMacro("__riscv_b", "93000");
+  if (ISAInfo.hasExtension("b"))
     Builder.defineMacro("__riscv_bitmanip");
-  }
 
-  if (HasV) {
-    Builder.defineMacro("__riscv_v", "10000");
+  if (ISAInfo.hasExtension("v"))
     Builder.defineMacro("__riscv_vector");
-  }
 
-  if (HasZba)
-    Builder.defineMacro("__riscv_zba", "93000");
-
-  if (HasZbb)
-    Builder.defineMacro("__riscv_zbb", "93000");
-
-  if (HasZbc)
-    Builder.defineMacro("__riscv_zbc", "93000");
-
-  if (HasZbe)
-    Builder.defineMacro("__riscv_zbe", "93000");
-
-  if (HasZbf)
-    Builder.defineMacro("__riscv_zbf", "93000");
-
-  if (HasZbm)
-    Builder.defineMacro("__riscv_zbm", "93000");
-
-  if (HasZbp)
-    Builder.defineMacro("__riscv_zbp", "93000");
-
-  if (HasZbproposedc)
-    Builder.defineMacro("__riscv_zbproposedc", "93000");
-
-  if (HasZbr)
-    Builder.defineMacro("__riscv_zbr", "93000");
-
-  if (HasZbs)
-    Builder.defineMacro("__riscv_zbs", "93000");
-
-  if (HasZbt)
-    Builder.defineMacro("__riscv_zbt", "93000");
-
-  if (HasZfh)
-    Builder.defineMacro("__riscv_zfh", "1000");
-
-  if (HasZvamo)
-    Builder.defineMacro("__riscv_zvamo", "10000");
-
-  if (HasZvlsseg)
-    Builder.defineMacro("__riscv_zvlsseg", "10000");
-}
-
 const Builtin::Info RISCVTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)                                               \
   {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
 #define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE)                               \
     {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE},
 #include "clang/Basic/BuiltinsRISCV.def"
 };
 
if (getTriple().getArch() == llvm::Triple::riscv64)
     Features["64bit"] = true;
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
 }
 
 /// Return true if has this feature, need to sync with handleTargetFeatures.
 bool RISCVTargetInfo::hasFeature(StringRef Feature) const {
   bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64;
-  return llvm::StringSwitch<bool>(Feature)
+  auto Result = llvm::StringSwitch<Optional<bool>>(Feature)
                     .Case("riscv", true)
                     .Case("riscv32", !Is64Bit)
                     .Case("riscv64", Is64Bit)
                     .Case("64bit", Is64Bit)
-      .Case("m", HasM)
-      .Case("a", HasA)
-      .Case("f", HasF)
-      .Case("d", HasD)
-      .Case("c", HasC)
-      .Case("experimental-b", HasB)
-      .Case("experimental-v", HasV)
-      .Case("experimental-zba", HasZba)
-      .Case("experimental-zbb", HasZbb)
-      .Case("experimental-zbc", HasZbc)
-      .Case("experimental-zbe", HasZbe)
-      .Case("experimental-zbf", HasZbf)
-      .Case("experimental-zbm", HasZbm)
-      .Case("experimental-zbp", HasZbp)
-      .Case("experimental-zbproposedc", HasZbproposedc)
-      .Case("experimental-zbr", HasZbr)
-      .Case("experimental-zbs", HasZbs)
-      .Case("experimental-zbt", HasZbt)
-      .Case("experimental-zfh", HasZfh)
-      .Case("experimental-zvamo", HasZvamo)
-      .Case("experimental-zvlsseg", HasZvlsseg)
-      .Default(false);
+                    .Default(None);
+  if (Result.hasValue())
+    return Result.getValue();
+
+  if (ISAInfo.isSupportedExtension(Feature,
+                                   /*CheckExperimental=*/true))
+    return ISAInfo.hasExtension(Feature);
+
+  return false;
 }
 
 /// Perform initialization based on the user configured set of features.
 bool RISCVTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
                                            DiagnosticsEngine &Diags) {
-  for (const auto &Feature : Features) {
-    if (Feature == "+m")
-      HasM = true;
-    else if (Feature == "+a")
-      HasA = true;
-    else if (Feature == "+f")
-      HasF = true;
-    else if (Feature == "+d")
-      HasD = true;
-    else if (Feature == "+c")
-      HasC = true;
-    else if (Feature == "+experimental-b")
-      HasB = true;
-    else if (Feature == "+experimental-v")
-      HasV = true;
-    else if (Feature == "+experimental-zba")
-      HasZba = true;
-    else if (Feature == "+experimental-zbb")
-      HasZbb = true;
-    else if (Feature == "+experimental-zbc")
-      HasZbc = true;
-    else if (Feature == "+experimental-zbe")
-      HasZbe = true;
-    else if (Feature == "+experimental-zbf")
-      HasZbf = true;
-    else if (Feature == "+experimental-zbm")
-      HasZbm = true;
-    else if (Feature == "+experimental-zbp")
-      HasZbp = true;
-    else if (Feature == "+experimental-zbproposedc")
-      HasZbproposedc = true;
-    else if (Feature == "+experimental-zbr")
-      HasZbr = true;
-    else if (Feature == "+experimental-zbs")
-      HasZbs = true;
-    else if (Feature == "+experimental-zbt")
-      HasZbt = true;
-    else if (Feature == "+experimental-zfh")
-      HasZfh = true;
-    else if (Feature == "+experimental-zvamo")
-      HasZvamo = true;
-    else if (Feature == "+experimental-zvlsseg")
-      HasZvlsseg = true;
-  }
+  unsigned XLEN = getTriple().isArch64Bit() ? 64 : 32;
+  ISAInfo.parse(XLEN, Features);
 
   return true;
 }
 
 bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
   return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name),
                                    /*Is64Bit=*/false);
 }

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

 //===--- RISCV.cpp - RISCV Helpers for Tools --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "RISCV.h"
+#include "ToolChains/CommonArgs.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
-#include "llvm/Option/ArgList.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
-#include "ToolChains/CommonArgs.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
-namespace {
-// Represents the major and version number components of a RISC-V extension
-struct RISCVExtensionVersion {
-  StringRef Major;
-  StringRef Minor;
-};
-} // end anonymous namespace
-
-static StringRef getExtensionTypeDesc(StringRef Ext) {
-  if (Ext.startswith("sx"))
-    return "non-standard supervisor-level extension";
-  if (Ext.startswith("s"))
-    return "standard supervisor-level extension";
-  if (Ext.startswith("x"))
-    return "non-standard user-level extension";
-  if (Ext.startswith("z"))
-    return "standard user-level extension";
-  return StringRef();
-}
-
-static StringRef getExtensionType(StringRef Ext) {
-  if (Ext.startswith("sx"))
-    return "sx";
-  if (Ext.startswith("s"))
-    return "s";
-  if (Ext.startswith("x"))
-    return "x";
-  if (Ext.startswith("z"))
-    return "z";
-  return StringRef();
-}
-
-// If the extension is supported as experimental, return the version of that
-// extension that the compiler currently supports.
-static Optional<RISCVExtensionVersion>
-isExperimentalExtension(StringRef Ext) {
-  if (Ext == "b" || Ext == "zba" || Ext == "zbb" || Ext == "zbc" ||
-      Ext == "zbe" || Ext == "zbf" || Ext == "zbm" || Ext == "zbp" ||
-      Ext == "zbr" || Ext == "zbs" || Ext == "zbt" || Ext == "zbproposedc")
-    return RISCVExtensionVersion{"0", "93"};
-  if (Ext == "v" || Ext == "zvamo" || Ext == "zvlsseg")
-    return RISCVExtensionVersion{"0", "10"};
-  if (Ext == "zfh")
-    return RISCVExtensionVersion{"0", "1"};
-  return None;
-}
-
-static bool isSupportedExtension(StringRef Ext) {
-  // LLVM supports "z" extensions which are marked as experimental.
-  if (isExperimentalExtension(Ext))
-    return true;
-
-  // LLVM does not support "sx", "s" nor "x" extensions.
-  return false;
-}
-
-// Extensions may have a version number, and may be separated by
-// an underscore '_' e.g.: rv32i2_m2.
-// Version number is divided into major and minor version numbers,
-// separated by a 'p'. If the minor version is 0 then 'p0' can be
-// omitted from the version string. E.g., rv32i2p0, rv32i2, rv32i2p1.
-static bool getExtensionVersion(const Driver &D, const ArgList &Args,
-                                StringRef MArch, StringRef Ext, StringRef In,
-                                std::string &Major, std::string &Minor) {
-  Major = std::string(In.take_while(isDigit));
-  In = In.substr(Major.size());
-
-  if (Major.size() && In.consume_front("p")) {
-    Minor = std::string(In.take_while(isDigit));
-    In = In.substr(Major.size() + 1);
-
-    // Expected 'p' to be followed by minor version number.
-    if (Minor.empty()) {
-      std::string Error =
-        "minor version number missing after 'p' for extension";
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-        << MArch << Error << Ext;
-      return false;
-    }
-  }
-
-  // Expected multi-character extension with version number to have no
-  // subsequent characters (i.e. must either end string or be followed by
-  // an underscore).
-  if (Ext.size() > 1 && In.size()) {
-    std::string Error =
-        "multi-character extensions must be separated by underscores";
-    D.Diag(diag::err_drv_invalid_riscv_ext_arch_name) << MArch << Error << In;
-    return false;
-  }
-
-  // If experimental extension, require use of current version number number
-  if (auto ExperimentalExtension = isExperimentalExtension(Ext)) {
-    if (!Args.hasArg(options::OPT_menable_experimental_extensions)) {
-      std::string Error =
-          "requires '-menable-experimental-extensions' for experimental extension";
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-          << MArch << Error << Ext;
-      return false;
-    } else if (Major.empty() && Minor.empty()) {
-      std::string Error =
-          "experimental extension requires explicit version number";
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-          << MArch << Error << Ext;
-      return false;
-    }
-    auto SupportedVers = *ExperimentalExtension;
-    if (Major != SupportedVers.Major || Minor != SupportedVers.Minor) {
-      std::string Error =
-          "unsupported version number " + Major;
-      if (!Minor.empty())
-        Error += "." + Minor;
-      Error += " for experimental extension (this compiler supports "
-            + SupportedVers.Major.str() + "."
-            + SupportedVers.Minor.str() + ")";
-
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-          << MArch << Error << Ext;
-      return false;
-    }
-    return true;
-  }
-
-  // Allow extensions to declare no version number
-  if (Major.empty() && Minor.empty())
-    return true;
-
-  // TODO: Handle supported extensions with version number.
-  std::string Error = "unsupported version number " + Major;
-  if (!Minor.empty())
-    Error += "." + Minor;
-  Error += " for extension";
-  D.Diag(diag::err_drv_invalid_riscv_ext_arch_name) << MArch << Error << Ext;
-
-  return false;
-}
-
-// 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', 'x', 's', 'sx' prefixes, follow a
-// canonical order, might have a version number (major, minor)
-// and are separated by a single underscore '_'.
-// Set the hardware features for the extensions that are supported.
-static void getExtensionFeatures(const Driver &D,
-                                 const ArgList &Args,
-                                 std::vector<StringRef> &Features,
-                                 StringRef &MArch, StringRef &Exts) {
-  if (Exts.empty())
-    return;
-
-  // Multi-letter extensions are seperated by a single underscore
-  // as described in RISC-V User-Level ISA V2.2.
-  SmallVector<StringRef, 8> Split;
-  Exts.split(Split, StringRef("_"));
-
-  SmallVector<StringRef, 4> Prefix{"z", "x", "s", "sx"};
-  auto I = Prefix.begin();
-  auto E = Prefix.end();
-
-  SmallVector<StringRef, 8> AllExts;
-
-  for (StringRef Ext : Split) {
-    if (Ext.empty()) {
-      D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-        << "extension name missing after separator '_'";
-      return;
-    }
-
-    StringRef Type = getExtensionType(Ext);
-    StringRef Desc = getExtensionTypeDesc(Ext);
-    auto Pos = Ext.find_if(isDigit);
-    StringRef Name(Ext.substr(0, Pos));
-    StringRef Vers(Ext.substr(Pos));
-
-    if (Type.empty()) {
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-        << MArch << "invalid extension prefix" << Ext;
-      return;
-    }
-
-    // Check ISA extensions are specified in the canonical order.
-    while (I != E && *I != Type)
-      ++I;
-
-    if (I == E) {
-      std::string Error = std::string(Desc);
-      Error += " not given in canonical order";
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-        << MArch <<  Error << Ext;
-      return;
-    }
-
-    // The order is OK, do not advance I to the next prefix
-    // to allow repeated extension type, e.g.: rv32ixabc_xdef.
-
-    if (Name.size() == Type.size()) {
-      std::string Error = std::string(Desc);
-      Error += " name missing after";
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-        << MArch << Error << Type;
-      return;
-    }
-
-    std::string Major, Minor;
-    if (!getExtensionVersion(D, Args, MArch, Name, Vers, Major, Minor))
-      return;
-
-    // Check if duplicated extension.
-    if (llvm::is_contained(AllExts, Name)) {
-      std::string Error = "duplicated ";
-      Error += Desc;
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-        << MArch << Error << Name;
-      return;
-    }
-
-    // Extension format is correct, keep parsing the extensions.
-    // TODO: Save Type, Name, Major, Minor to avoid parsing them later.
-    AllExts.push_back(Name);
-  }
-
-  // Set target features.
-  // TODO: Hardware features to be handled in Support/TargetParser.cpp.
-  // TODO: Use version number when setting target features.
-  for (auto Ext : AllExts) {
-    if (!isSupportedExtension(Ext)) {
-      StringRef Desc = getExtensionTypeDesc(getExtensionType(Ext));
-      std::string Error = "unsupported ";
-      Error += Desc;
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-        << MArch << Error << Ext;
-      return;
-    }
-    if (Ext == "zvlsseg") {
-      Features.push_back("+experimental-v");
-      Features.push_back("+experimental-zvlsseg");
-    } else if (Ext == "zvamo") {
-      Features.push_back("+experimental-v");
-      Features.push_back("+experimental-zvlsseg");
-      Features.push_back("+experimental-zvamo");
-    } else if (isExperimentalExtension(Ext))
-      Features.push_back(Args.MakeArgString("+experimental-" + Ext));
-    else
-      Features.push_back(Args.MakeArgString("+" + Ext));
-  }
-}
-
 // Returns false if an error is diagnosed.
-static bool getArchFeatures(const Driver &D, StringRef MArch,
+static bool getArchFeatures(const Driver &D, StringRef Arch,
                             std::vector<StringRef> &Features,
                             const ArgList &Args) {
-  // RISC-V ISA strings must be lowercase.
-  if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
-    D.Diag(diag::err_drv_invalid_riscv_arch_name)
-        << MArch << "string must be lowercase";
-    return false;
-  }
-
-  // ISA string must begin with rv32 or rv64.
-  if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
-      (MArch.size() < 5)) {
-    D.Diag(diag::err_drv_invalid_riscv_arch_name)
-        << MArch << "string must begin with rv32{i,e,g} or rv64{i,g}";
-    return false;
-  }
+  llvm::RISCVISAInfo ISAInfo;
 
-  bool HasRV64 = MArch.startswith("rv64");
+  bool EnableExperimentalExtensions =
+      Args.hasArg(options::OPT_menable_experimental_extensions);
 
-  // The canonical order specified in ISA manual.
-  // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
-  StringRef StdExts = "mafdqlcbjtpvn";
-  bool HasF = false, HasD = false;
-  char Baseline = MArch[4];
-
-  // First letter should be 'e', 'i' or 'g'.
-  switch (Baseline) {
-  default:
+  if (auto E = ISAInfo.parse(Arch, EnableExperimentalExtensions)) {
+    handleAllErrors(std::move(E), [&](llvm::StringError &ErrMsg) {
       D.Diag(diag::err_drv_invalid_riscv_arch_name)
-        << MArch << "first letter should be 'e', 'i' or 'g'";
-    return false;
-  case 'e': {
-    StringRef Error;
-    // Currently LLVM does not support 'e'.
-    // Extension 'e' is not allowed in rv64.
-    if (HasRV64)
-      Error = "standard user-level extension 'e' requires 'rv32'";
-    else
-      Error = "unsupported standard user-level extension 'e'";
-    D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch << Error;
-    return false;
-  }
-  case 'i':
-    break;
-  case 'g':
-    // g = imafd
-    StdExts = StdExts.drop_front(4);
-    Features.push_back("+m");
-    Features.push_back("+a");
-    Features.push_back("+f");
-    Features.push_back("+d");
-    HasF = true;
-    HasD = true;
-    break;
-  }
-
-  // Skip rvxxx
-  StringRef Exts = MArch.substr(5);
-
-  // Remove multi-letter standard extensions, non-standard extensions and
-  // supervisor-level extensions. They have 'z', 'x', 's', 'sx' 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);
-  }
-
-  std::string Major, Minor;
-  if (!getExtensionVersion(D, Args, MArch, std::string(1, Baseline), Exts,
-                           Major, Minor))
-    return false;
+          << Arch << ErrMsg.getMessage();
+    });
 
-  // Consume the base ISA version number and any '_' between rvxxx and the
-  // first extension
-  Exts = Exts.drop_front(Major.size());
-  if (!Minor.empty())
-    Exts = Exts.drop_front(Minor.size() + 1 /*'p'*/);
-  Exts.consume_front("_");
-
-  // TODO: Use version number when setting target features
-
-  auto StdExtsItr = StdExts.begin();
-  auto StdExtsEnd = StdExts.end();
-
-  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.
-      StringRef Error;
-      if (StdExts.contains(c))
-        Error = "standard user-level extension not given in canonical order";
-      else
-        Error = "invalid standard user-level extension";
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-          << MArch << Error << std::string(1, c);
-      return false;
-    }
 
-    // Move to next char to prevent repeated letter.
-    ++StdExtsItr;
-
-    std::string Next, Major, Minor;
-    if (std::next(I) != E)
-      Next = std::string(std::next(I), E);
-    if (!getExtensionVersion(D, Args, MArch, std::string(1, c), Next, Major,
-                             Minor))
-      return false;
-
-    // The order is OK, then push it into features.
-    // TODO: Use version number when setting target features
-    switch (c) {
-    default:
-      // Currently LLVM supports only "mafdc".
-      D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)
-          << MArch << "unsupported standard user-level extension"
-          << std::string(1, c);
-      return false;
-    case 'm':
-      Features.push_back("+m");
-      break;
-    case 'a':
-      Features.push_back("+a");
-      break;
-    case 'f':
-      Features.push_back("+f");
-      HasF = true;
-      break;
-    case 'd':
-      Features.push_back("+d");
-      HasD = true;
-      break;
-    case 'c':
-      Features.push_back("+c");
-      break;
-    case 'b':
-      Features.push_back("+experimental-b");
-      Features.push_back("+experimental-zba");
-      Features.push_back("+experimental-zbb");
-      Features.push_back("+experimental-zbc");
-      Features.push_back("+experimental-zbe");
-      Features.push_back("+experimental-zbf");
-      Features.push_back("+experimental-zbm");
-      Features.push_back("+experimental-zbp");
-      Features.push_back("+experimental-zbr");
-      Features.push_back("+experimental-zbs");
-      Features.push_back("+experimental-zbt");
-      break;
-    case 'v':
-      Features.push_back("+experimental-v");
-      Features.push_back("+experimental-zvlsseg");
-      break;
-    }
-
-    // Consume full extension name and version, including any optional '_'
-    // between this extension and the next
-    ++I;
-    I += Major.size();
-    if (Minor.size())
-      I += Minor.size() + 1 /*'p'*/;
-    if (*I == '_')
-      ++I;
-  }
-
-  // Dependency check.
-  // It's illegal to specify the 'd' (double-precision floating point)
-  // extension without also specifying the 'f' (single precision
-  // floating-point) extension.
-  if (HasD && !HasF) {
-    D.Diag(diag::err_drv_invalid_riscv_arch_name)
-        << MArch << "d requires f extension to also be specified";
-    return false;
-  }
-
-  // Additional dependency checks.
-  // TODO: The 'q' extension requires rv64.
-  // TODO: It is illegal to specify 'e' extensions with 'f' and 'd'.
-
-  // Handle all other types of extensions.
-  getExtensionFeatures(D, Args, Features, MArch, OtherExts);
-
+  ISAInfo.toFeatures(Args, Features);
   return true;
 }
 
 // Get features except standard extension feature
 static void getRISCFeaturesFromMcpu(const Driver &D, const llvm::Triple &Triple,
                                     const llvm::opt::ArgList &Args,
                                     const llvm::opt::Arg *A, StringRef Mcpu,
                                     std::vector<StringRef> &Features) {
StringRef riscv::getRISCVABI(const ArgList &Args, const llvm::Triple &Triple) {
   // rv32g | rv32*d -> ilp32d
   // rv32e -> ilp32e
   // rv32* -> ilp32
   // rv64g | rv64*d -> lp64d
   // rv64* -> lp64
   StringRef MArch = getRISCVArch(Args, Triple);
 
   if (MArch.startswith_insensitive("rv32")) {
     // FIXME: parse `March` to find `D` extension properly
     if (MArch.substr(4).contains_insensitive("d") ||
         MArch.startswith_insensitive("rv32g"))
       return "ilp32d";
     else if (MArch.startswith_insensitive("rv32e"))
       return "ilp32e";
     else
       return "ilp32";
   } else if (MArch.startswith_insensitive("rv64")) {
     // FIXME: parse `March` to find `D` extension properly
     if (MArch.substr(4).contains_insensitive("d") ||
         MArch.startswith_insensitive("rv64g"))
       return "lp64d";
     else
       return "lp64";
   }
 
   // 3. Choose a default based on the triple
   //
   // We deviate from GCC's defaults here:
   // - On `riscv{XLEN}-unknown-elf` we use the integer calling convention only.
   // - On all other OSs we use the double floating point calling convention.
   if (Triple.getArch() == llvm::Triple::riscv32) {
     if (Triple.getOS() == llvm::Triple::UnknownOS)
       return "ilp32";
     else
       return "ilp32d";
   } else {
     if (Triple.getOS() == llvm::Triple::UnknownOS)
       return "lp64";
     else
       return "lp64d";
   }
 }
 
 StringRef riscv::getRISCVArch(const llvm::opt::ArgList &Args,
                               const llvm::Triple &Triple) {
   assert((Triple.getArch() == llvm::Triple::riscv32 ||
           Triple.getArch() == llvm::Triple::riscv64) &&
          "Unexpected triple");
 
   // GCC's logic around choosing a default `-march=` is complex. If GCC is not

clang/test/Driver/riscv-arch.c

 // RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i2p0 -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s

 // CHECK-ILP32D:      "-target-feature" "+m"
 // CHECK-ILP32D-SAME: {{^}} "-target-feature" "+a"
 // CHECK-ILP32D-SAME: {{^}} "-target-feature" "+f"
 // CHECK-ILP32D-SAME: {{^}} "-target-feature" "+d"
 // CHECK-ILP32D-SAME: {{^}} "-target-feature" "+c"
 
 // RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i2p0 -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s

 
 // RUN: %clang -target riscv64-unknown-elf -march=RV64I -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64I-UPPER %s
 // RV64I-UPPER: error: invalid arch name 'RV64I'
 
 
 // Testing specific messages and unsupported extensions.
 
 // RUN: %clang -target riscv32-unknown-elf -march=rv32e -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32E %s
 // RV32E: error: invalid arch name 'rv32e',
 // RV32E: standard user-level extension 'e'
 
 // RUN: %clang -target riscv64-unknown-elf -march=rv64e -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64E %s
 // RV64E: error: invalid arch name 'rv64e',
 // RV64E: standard user-level extension 'e' requires 'rv32'
 
 // RUN: %clang -target riscv32-unknown-elf -march=rv32imC -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-LOWER %s

 // RV32-CVER: error: invalid arch name 'rv32imc5', unsupported
 // RV32-CVER: version number 5 for extension 'c'
 
 // RUN: %clang -target riscv32-unknown-elf -march=rv32i2p -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-IMINOR-MISS %s
 // RV32-IMINOR-MISS: error: invalid arch name 'rv32i2p',
 // RV32-IMINOR-MISS: minor version number missing after 'p' for extension 'i'
 
-// RUN: %clang -target riscv32-unknown-elf -march=rv32i2p0 -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-IMINOR0 %s
-// RV32-IMINOR0: error: invalid arch name 'rv32i2p0',
-// RV32-IMINOR0: unsupported version number 2.0 for extension 'i'
-
 // RUN: %clang -target riscv32-unknown-elf -march=rv32i2p1 -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-IMINOR1 %s
 // RV32-IMINOR1: error: invalid arch name 'rv32i2p1', unsupported
 // RV32-IMINOR1: version number 2.1 for extension 'i'
 
 // RUN: %clang -target riscv32-unknown-elf -march=rv32ixt2p -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XMINOR-MISS %s
 // RV32-XMINOR-MISS: error: invalid arch name 'rv32ixt2p',

llvm/include/llvm/Support/RISCVISAInfo.h

+//===-- RISCVArchStringParser.h - RISCV Arch String Parser ------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_RISCVISAINFO_H
+#define LLVM_SUPPORT_RISCVISAINFO_H
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/Error.h"
+
+#include <map>
+#include <string>
+#include <vector>
+
+namespace llvm {
+struct RISCVExtensionInfo {
+  std::string ExtName;
+  unsigned MajorVersion;
+  unsigned MinorVersion;
+};
+
+class RISCVISAInfo {
+public:
+  RISCVISAInfo();
+
+  static bool compareExtension(const std::string &LHS, const std::string &RHS);
+  // Helper class for OrderedExtensionMap.
+  struct ExtensionComparator {
+    bool operator()(const std::string &LHS, const std::string &RHS) const {
+      return compareExtension(LHS, RHS);
+    }
+  };
+  // OrderedExtensionMap is a StringMap-like container, but specialized to
+  // keep entries in canonical order of extension.
+  typedef std::map<std::string, RISCVExtensionInfo, ExtensionComparator>
+      OrderedExtensionMap;
+
+  // Parse RISCV ISA info from arch string.
+  Error parse(StringRef Arch, bool EnableExperimentalExtension,
+              bool ExperimentalExtensionVersionCheck = true);
+
+  // Parse RISCV ISA info from feature vector.
+  void parse(unsigned XLen, const std::vector<std::string> &Features);
+
+  // Convert RISCV ISA info to a feature vector.
+  void toFeatures(const llvm::opt::ArgList &Args,
+                  std::vector<StringRef> &Features) const;
+
+  const OrderedExtensionMap &getExtensions() const { return Exts; };
+
+  unsigned getXLen() const { return XLen; };
+  unsigned getFLen() const { return FLen; };
+
+  bool hasExtension(StringRef Ext) const;
+  std::string toString() const;
+
+  static bool isSupportedExtension(StringRef Ext, bool CheckExperimental);
+  static bool isSupportedExtension(StringRef Ext, unsigned MajorVersion,
+                                   unsigned MinorVersion,
+                                   bool CheckExperimental);
+
+private:
+  unsigned XLen;
+  unsigned FLen;
+
+  OrderedExtensionMap Exts;
+
+  void addExtension(StringRef ExtName, unsigned MajorVersion = 0,
+                    unsigned MinorVersion = 0);
+
+  void updateFLen();
+};
+
+} // namespace llvm
+
+#endif

llvm/lib/Support/CMakeLists.txt

add_llvm_component_library(LLVMSupport
   Optional.cpp
   Parallel.cpp
   PluginLoader.cpp
   PrettyStackTrace.cpp
   RandomNumberGenerator.cpp
   Regex.cpp
   RISCVAttributes.cpp
   RISCVAttributeParser.cpp
+  RISCVISAInfo.cpp
   ScaledNumber.cpp
   ScopedPrinter.cpp
   SHA1.cpp
   SHA256.cpp
   Signposts.cpp
   SmallPtrSet.cpp
   SmallVector.cpp
   SourceMgr.cpp

llvm/lib/Support/RISCVISAInfo.cpp

+//===-- RISCVISAInfo.cpp - RISCV Arch String Parser --------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/RISCVISAInfo.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <array>
+#include <string>
+#include <vector>
+
+using namespace llvm;
+
+namespace {
+// Represents the major and version number components of a RISC-V extension
+struct RISCVExtensionVersion {
+  unsigned Major;
+  unsigned Minor;
+};
+
+struct RISCVSupportedExtensionInfo {
+  const char *Name;
+  // Supported version.
+  RISCVExtensionVersion Version;
+  // Experimental extension?
+  bool Experimental;
+};
+
+} // end anonymous namespace
+
+static const StringRef AllStdExts = "mafdqlcbjtpvn";
+
+static const RISCVSupportedExtensionInfo SupportedExtensionInfos[] = {
+    {"i", RISCVExtensionVersion{2, 0}, /* Experimental */ false},
+    {"e", RISCVExtensionVersion{1, 9}, /* Experimental */ false},
+    {"m", RISCVExtensionVersion{2, 0}, /* Experimental */ false},
+    {"a", RISCVExtensionVersion{2, 0}, /* Experimental */ false},
+    {"f", RISCVExtensionVersion{2, 0}, /* Experimental */ false},
+    {"d", RISCVExtensionVersion{2, 0}, /* Experimental */ false},
+    {"c", RISCVExtensionVersion{2, 0}, /* Experimental */ false},
+    {"b", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"v", RISCVExtensionVersion{0, 10}, /* Experimental */ true},
+
+    {"zba", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbb", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbc", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbe", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbf", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbm", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbp", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbr", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbs", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbt", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+    {"zbproposedc", RISCVExtensionVersion{0, 93}, /* Experimental */ true},
+
+    {"zvamo", RISCVExtensionVersion{0, 10}, /* Experimental */ true},
+    {"zvlsseg", RISCVExtensionVersion{0, 10}, /* Experimental */ true},
+
+    {"zfh", RISCVExtensionVersion{0, 1}, /* Experimental */ true},
+};
+
+// Remove 'experimental-' if it's prefix of string,
+// return true if did the strip.
+static bool stripExperimentalPrefix(StringRef &Ext) {
+  return Ext.consume_front("experimental-");
+}
+
+RISCVISAInfo::RISCVISAInfo() : XLen(0), FLen(0) {}
+
+void RISCVISAInfo::addExtension(StringRef ExtName, unsigned MajorVersion,
+                                unsigned MinorVersion) {
+  RISCVExtensionInfo Ext;
+  Ext.ExtName = ExtName.str();
+  Ext.MajorVersion = MajorVersion;
+  Ext.MinorVersion = MinorVersion;
+  Exts[ExtName.str()] = Ext;
+}
+
+static StringRef getExtensionTypeDesc(StringRef Ext) {
+  if (Ext.startswith("sx"))
+    return "non-standard supervisor-level extension";
+  if (Ext.startswith("s"))
+    return "standard supervisor-level extension";
+  if (Ext.startswith("x"))
+    return "non-standard user-level extension";
+  if (Ext.startswith("z"))
+    return "standard user-level extension";
+  return StringRef();
+}
+
+static StringRef getExtensionType(StringRef Ext) {
+  if (Ext.startswith("sx"))
+    return "sx";
+  if (Ext.startswith("s"))
+    return "s";
+  if (Ext.startswith("x"))
+    return "x";
+  if (Ext.startswith("z"))
+    return "z";
+  return StringRef();
+}
+
+// Helper function for fiilter SupportedExtensionInfos by name.
+static auto filterSupportedExtensionInfosByName(StringRef ExtName) {
+  return make_filter_range(SupportedExtensionInfos,
+                           [&](const RISCVSupportedExtensionInfo &ExtInfo) {
+                             return ExtInfo.Name == ExtName;
+                           });
+}
+
+static Optional<RISCVExtensionVersion> isExperimentalExtension(StringRef Ext) {
+  for (auto const &SupportedExtensionInfo :
+       filterSupportedExtensionInfosByName(Ext)) {
+    if (!SupportedExtensionInfo.Experimental)
+      continue;
+
+    return SupportedExtensionInfo.Version;
+  }
+  return None;
+}
+
+bool RISCVISAInfo::isSupportedExtension(StringRef Ext, bool CheckExperimental) {
+  bool IsExperimental = false;
+  if (CheckExperimental)
+    IsExperimental = stripExperimentalPrefix(Ext);
+
+  for (auto const &SupportedExtensionInfo :
+       filterSupportedExtensionInfosByName(Ext)) {
+    if (CheckExperimental)
+      return SupportedExtensionInfo.Experimental == IsExperimental;
+    return true;
+  }
+  return false;
+}
+
+bool RISCVISAInfo::isSupportedExtension(StringRef Ext, unsigned MajorVersion,
+                                        unsigned MinorVersion,
+                                        bool CheckExperimental) {
+  bool IsExperimental = false;
+  if (CheckExperimental)
+    IsExperimental = stripExperimentalPrefix(Ext);
+
+  for (auto const &SupportedExtensionInfo :
+       filterSupportedExtensionInfosByName(Ext)) {
+    if (CheckExperimental &&
+        SupportedExtensionInfo.Experimental != IsExperimental)
+      continue;
+    if ((MajorVersion != SupportedExtensionInfo.Version.Major) ||
+        (MinorVersion != SupportedExtensionInfo.Version.Minor))
+      continue;
+
+    return true;
+  }
+  return false;
+}
+
+bool RISCVISAInfo::hasExtension(StringRef Ext) const {
+  stripExperimentalPrefix(Ext);
+
+  if (!isSupportedExtension(Ext, /* CheckExperimental */ false))
+    return false;
+
+  return Exts.count(Ext.str()) != 0;
+}
+
+// Get the rank for single-letter extension, lower value meaning higher
+// priority.
+static int singleLetterExtensionRank(char Ext) {
+  switch (Ext) {
+  case 'i':
+    return 0;
+  case 'e':
+    return 1;
+  default:
+    break;
+  }
+
+  size_t Pos = AllStdExts.find(Ext);
+  int Rank;
+  if (Pos == StringRef::npos)
+    // If got an unknown extension letter, then give it an alphabetical
+    // order, but after all known standard extension.
+    Rank = AllStdExts.size() + (Ext - 'a');
+  else
+    // e and i has higher Rank.
+    Rank = Pos + 2;
+
+  return Rank;
+}
+
+// Get the rank for multi-letter extension, lower value meaning higher
+// priority/order in canonical order.
+static int multiLetterExtensionRank(const std::string &ExtName) {
+  assert(ExtName.length() >= 2);
+  int HighOrder;
+  int LowOrder = 0;
+  // The order between multi-char extensions: s -> h -> z -> x.
+  char ExtClass = ExtName[0];
+  switch (ExtClass) {
+  case 's':
+    HighOrder = 0;
+    break;
+  case 'h':
+    HighOrder = 1;
+    break;
+  case 'z':
+    HighOrder = 2;
+    break;
+  case 'x':
+    HighOrder = 3;
+    break;
+  default:
+    assert(false && "Unknown prefix for multi-char extension");
+    return -1;
+  }
+
+  // `z` extension must be sorted by canonical order of second letter.
+  // e.g. zmx has higher rank than zax.
+  if (ExtClass == 'z')
+    LowOrder = singleLetterExtensionRank(ExtName[1]);
+
+  return (HighOrder << 8) + LowOrder;
+}
+
+// Compare function for extension.
+// Only compare the extension name, ignore version comparesion.
+bool RISCVISAInfo::compareExtension(const std::string &LHS,
+                                    const std::string &RHS) {
+  size_t LHSLen = LHS.length();
+  size_t RHSLen = RHS.length();
+  int LHSRank, RHSRank;
+  if (LHSLen == 1 && RHSLen != 1)
+    return true;
+
+  if (LHSLen != 1 && RHSLen == 1)
+    return false;
+
+  if (LHSLen == 1 && RHSLen == 1) {
+    LHSRank = singleLetterExtensionRank(LHS[0]);
+    RHSRank = singleLetterExtensionRank(RHS[0]);
+
+    return LHSRank < RHSRank;
+  }
+
+  // Both are multi-char ext here.
+  LHSRank = multiLetterExtensionRank(LHS);
+  RHSRank = multiLetterExtensionRank(RHS);
+  if (LHSRank != RHSRank)
+    return LHSRank < RHSRank;
+
+  // If the rank is same, it must be sorted by lexical order.
+  return LHS < RHS;
+}
+
+void RISCVISAInfo::toFeatures(const llvm::opt::ArgList &Args,
+                              std::vector<StringRef> &Features) const {
+  for (auto &Ext : Exts) {
+    StringRef ExtName = Ext.first;
+
+    if (ExtName == "i")
+      continue;
+
+    if (ExtName == "zvlsseg") {
+      Features.push_back("+experimental-v");
+      Features.push_back("+experimental-zvlsseg");
+    } else if (ExtName == "zvamo") {
+      Features.push_back("+experimental-v");
+      Features.push_back("+experimental-zvlsseg");
+      Features.push_back("+experimental-zvamo");
+    } else if (isExperimentalExtension(ExtName))
+      Features.push_back(Args.MakeArgString("+experimental-" + ExtName));
+    else
+      Features.push_back(Args.MakeArgString("+" + ExtName));
+  }
+}
+
+// Extensions may have a version number, and may be separated by
+// an underscore '_' e.g.: rv32i2_m2.
+// Version number is divided into major and minor version numbers,
+// separated by a 'p'. If the minor version is 0 then 'p0' can be
+// omitted from the version string. E.g., rv32i2p0, rv32i2, rv32i2p1.
+static Error getExtensionVersion(StringRef MArch, StringRef Ext, StringRef In,
+                                 unsigned &Major, unsigned &Minor,
+                                 unsigned &ConsumeLength,
+                                 bool EnableExperimentalExtension,
+                                 bool ExperimentalExtensionVersionCheck) {
+  std::string MajorStr, MinorStr;
+  Major = 0;
+  Minor = 0;
+  ConsumeLength = 0;
+  MajorStr = std::string(In.take_while(isDigit));
+  In = In.substr(MajorStr.size());
+
+  if (MajorStr.size() && In.consume_front("p")) {
+    MinorStr = std::string(In.take_while(isDigit));
+    In = In.substr(MajorStr.size() + 1);
+
+    // Expected 'p' to be followed by minor version number.
+    if (MinorStr.empty()) {
+      return createStringError(
+          errc::invalid_argument,
+          "minor version number missing after 'p' for extension '%s'",
+          Ext.str().c_str());
+    }
+  }
+
+  if (!MajorStr.empty() && StringRef(MajorStr).getAsInteger(10, Major))
+    return createStringError(
+        errc::invalid_argument,
+        "Failed to parsing major version number for extension '%s'",
+        Ext.str().c_str());
+
+  if (!MinorStr.empty() && StringRef(MinorStr).getAsInteger(10, Minor))
+    return createStringError(
+        errc::invalid_argument,
+        "Failed to parsing minor version number for extension '%s'",
+        Ext.str().c_str());
+
+  ConsumeLength = MajorStr.length