[lld][RISCV] Avoid error when encountering unrecognised ISA extensions/versions in RISC-V attributes

[asb]

Summary

This patch follows on from this RFC thread https://discourse.llvm.org/t/rfc-resolving-issues-related-to-extension-versioning-in-risc-v/68472/ and the ensuing discussion in the RISC-V LLVM sync-up call. The consensus agreed that the behaviour change in LLD introduced in D138550 that results in object files including arch attributes with unrecognised extensions or versions of extensions is a regression and should be treated as such. As it stands, this logic means that LLD will error out if trying to link a RISC-V object file from LLVM HEAD (rv32i2p0/rv64i2p0) with one from current GCC (rv32i2p1/rv64i2p1 by default).

There's a bigger discussion about exactly when to warn vs error and so on, and how to control that, and this patch doesn't attempt to address all those questions. It simply tries to fix the problem with a minimally invasive change, intended to be cherry-picked for 16.0.x (ideally 16.0.0, but queued for 16.0.1 if we're too late in the release cycle).

As you can see from the test changes, although the changed logic is mostly more permissive, it will reject some embedded arch strings that were accepted before. Because the same logic was previously used for parsing human-written -march as for the arch attribute (intended to be stored in normalised form), various short-hand formats were previously accepted. Maintaining support for such ill-formed attributes would substantially complicate the logic, and given the previous implementation was so much stricter in every other way, was surely a bug rather than a design choice.

Surprisingly, the precise rules for how numbers can be embedded into extension names isn't fully defined (there is a PR to the ISA manual that is not yet merged https://github.com/riscv/riscv-isa-manual/pull/718). In the absence of specific criteria for rejecting extensions names that would be ambiguous in abbreviated form, RISCVISAInfo::parseArchStringNormalized just pulls out the version information from the end of each extension description.

[asb]

An important additional clarification: not errorring or warning when encountering unrecognised extensions or extension versions (as implemented in this patch) matches current binutils ld behaviour.

$ cat a.s
.section .riscv.attributes,"",@0x70000003
.byte 0x41
.long .Lend-.riscv.attributes-1
.asciz "riscv"  # vendor
.Lbegin:
.byte 1  # Tag_File
.long .Lend-.Lbegin
.byte 5  # Tag_RISCV_arch
.asciz "rv64i2p0_zmadeup1p0"
.Lend:

$ cat b.s
.section .riscv.attributes,"",@0x70000003
.byte 0x41
.long .Lend-.riscv.attributes-1
.asciz "riscv"  # vendor
.Lbegin:
.byte 1  # Tag_File
.long .Lend-.Lbegin
.byte 5  # Tag_RISCV_arch
.asciz "rv64i50p0_zanotherfake1p0"
.Lend:

$ riscv64-linux-gnu-as a.s -o a.o
$ riscv64-linux-gnu-as b.s -o b.o
$ riscv64-linux-gnu-ld a.o b.o
riscv64-linux-gnu-ld: warning: cannot find entry symbol _start; defaulting to 0000000000010078
$ llvm-readobj --arch-specific a.out
File: a.out
Format: elf64-littleriscv
Arch: riscv64
AddressSize: 64bit
LoadName: <Not found>
BuildAttributes {
  FormatVersion: 0x41
  Section 1 {
    SectionLength: 53
    Vendor: riscv
    Tag: Tag_File (0x1)
    Size: 43
    FileAttributes {
      Attribute {
        Tag: 5
        TagName: arch
        Value: rv64i50p0_zmadeup1p0_zanotherfake1p0
      }
    }
  }
}

[reames]

I don't feel comfortable signing off on this as I don't know this part of code well enough, but I want to chime in with support for treating this like a regression. If we can get a change merged into the release branch without compromising quality in other ways, I think we should.

Alex, on the code, can you explain why you took the approach of writing a new parser specifically for this case? The existing routine has the IgnoreUnknown flag. How does that differ from the semantics you need here?

[asb]

In D144353#4145824, @reames wrote:

Alex, on the code, can you explain why you took the approach of writing a new parser specifically for this case? The existing routine has the IgnoreUnknown flag. How does that differ from the semantics you need here?

That's a good question and a consideration I should have mentioned in the patch description. The issue with IgnoreUnknown is the extension is literally ignored - it isn't added to the returned RISCVISAInfo, and so will be silently dropped rather than merged according to the merging rules implemented in LLD and specified in the psABI. There's also the fact that the existing routine accepts a much wider range of inputs (and performs additional normalisation) than is required/desired for the inputs from the ELF attributes - though if parseArchString with IgnoreUnknown was closer to what we want, I'd have strongly considered just using that as a stop-gap.

[MaskRay]

LG and I support back porting this to the release/16.x branch.

I think the new API needs a unittest under llvm/unittests/Support/, which the original patch adding parseArchString omitted.

lld/test/ELF/riscv-attributes.s

18–19

ditto below

[MaskRay]

llvm/lib/Support/RISCVISAInfo.cpp

526

This is untested. You can test it with a test under llvm/unittest/Support

552

getExtensionVersion uses unsigned instead of unsigned long long.

562

Merge this into the loop condition

565

pre-increment

[kito-cheng]

LGTM

I like the idea which decoupling the parsing logic of the normal march string parsing, and we can have a simple parser logic here since ELF attribute has specified that should have explicitly version number and separated by _.

This also enable us could implement further complicated merge rule like merging unknown extension, different extension version and unsupported extension version - not just limited to handle knowing extension and supported version.

[reames]

Reverse ping. We've got two LGTMs and this is a serious regression. Is there anything holding this back?

[asb]

I'll post an updated version with the requested C++ unit tests shortly. I understood MaskRay's LGTM to be conditional on addressing those comments.

llvm/lib/Support/RISCVISAInfo.cpp

552

unsigned long long needs to be used for StringRef::getAsUnsignedInteger. Changing to unsigned gives:

/home/asb/llvm-project/llvm/lib/Support/RISCVISAInfo.cpp:552:9: error: no matching function for call to 'getAsUnsignedInteger'
    if (getAsUnsignedInteger(MinorVersionStr, 10, MinorVersion))
        ^~~~~~~~~~~~~~~~~~~~
/home/asb/llvm-project/llvm/include/llvm/ADT/StringRef.h:34:8: note: candidate function not viable: no known conversion from 'unsigned int' to 'unsigned long long &' for 3rd argument
  bool getAsUnsignedInteger(StringRef Str, unsigned Radix,
       ^

[asb]

Addresses review comments and adds C++ unit test.

[asb]

If anyone can confirm there's nothing egregiously wrong (or obvious gaps) with the unit test and other review feedback responses I'd appreciate it. I interpreted the reviews as LGTM or LGTM once minor comments are addressed, without necessarily requiring another review loop. So given that and the time pressure for the backport request, I'll commit tomorrow morning UK time unless there are objections or further review comments.

[MaskRay]

llvm/lib/Support/RISCVISAInfo.cpp

552

OK. We can use the template member function StringRef::getAsInteger to avoid the implicit unsigned long long to unsigned cast below.

llvm/unittests/Support/RISCVISAInfoTest.cpp

2

//===-- or 3 dashes is more common

23

s/auto/StringRef/

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

31

s/auto/StringRef/

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

39

s/auto/StringRef/

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

52

Use EXPECT_ for non-fatal checks.

68

Use EXPECT_ for non-fatal checks.

76

Use EXPECT_ for non-fatal checks.

87

Use EXPECT_ for non-fatal checks.

102

Use EXPECT_ for non-fatal checks.

[asb]

Address review comments (thanks!)

llvm/unittests/Support/RISCVISAInfoTest.cpp

23

I've gone ahead and changed these, but I do consider this a case of "the type is already obvious from the context" and hence in-line with the recommendation. Clearly not a big deal either way.

lld/ELF/Arch/RISCV.cpp

public:
   DenseMap<unsigned, StringRef> strAttr;
   size_t size = 0;
 };
 } // namespace
 
 static void mergeArch(RISCVISAInfo::OrderedExtensionMap &mergedExts,
                       unsigned &mergedXlen, const InputSectionBase *sec,
                       StringRef s) {
-  auto maybeInfo =
-      RISCVISAInfo::parseArchString(s, /*EnableExperimentalExtension=*/true,
-                                    /*ExperimentalExtensionVersionCheck=*/true);
+  auto maybeInfo = RISCVISAInfo::parseNormalizedArchString(s);
   if (!maybeInfo) {
     errorOrWarn(toString(sec) + ": " + s + ": " +
                 llvm::toString(maybeInfo.takeError()));
     return;
   }
 
   // Merge extensions.
   RISCVISAInfo &info = **maybeInfo;
   if (mergedExts.empty()) {
     mergedExts = info.getExtensions();
     mergedXlen = info.getXLen();
   } else {
     for (const auto &ext : info.getExtensions()) {
       if (auto it = mergedExts.find(ext.first); it != mergedExts.end()) {
-        // TODO This is untested because RISCVISAInfo::parseArchString does not
-        // accept unsupported versions yet.
         if (std::tie(it->second.MajorVersion, it->second.MinorVersion) >=
             std::tie(ext.second.MajorVersion, ext.second.MinorVersion))
           continue;
       }
       mergedExts[ext.first] = ext.second;
     }
   }
 }

lld/test/ELF/riscv-attributes.s

 # RUN: llvm-readobj --arch-specific out1 | FileCheck %s
 
 # RUN: llvm-mc -filetype=obj -triple=riscv64 b.s -o b.o
 # RUN: llvm-mc -filetype=obj -triple=riscv64 c.s -o c.o
 # RUN: ld.lld a.o b.o c.o -o out2
 # RUN: llvm-readobj --arch-specific out2 | FileCheck %s --check-prefix=CHECK2
 
 # RUN: llvm-mc -filetype=obj -triple=riscv64 unrecognized_ext1.s -o unrecognized_ext1.o
-# RUN: not ld.lld unrecognized_ext1.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=UNRECOGNIZED_EXT1 --implicit-check-not=error:
-# UNRECOGNIZED_EXT1: error: unrecognized_ext1.o:(.riscv.attributes): rv64i2p0_y2p0: invalid standard user-level extension 'y'
+# RUN: ld.lld -e 0 unrecognized_ext1.o -o unrecognized_ext1 2>&1 | count 0
+# RUN: llvm-readobj --arch-specific unrecognized_ext1 | FileCheck %s --check-prefix=UNRECOGNIZED_EXT1
 
 # RUN: llvm-mc -filetype=obj -triple=riscv64 unrecognized_ext2.s -o unrecognized_ext2.o
-# RUN: not ld.lld unrecognized_ext2.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=UNRECOGNIZED_EXT2 --implicit-check-not=error:
-# UNRECOGNIZED_EXT2: error: unrecognized_ext2.o:(.riscv.attributes): rv64i2p0_zmadeup1p0: unsupported version number 1.0 for extension 'zmadeup'
+# RUN: ld.lld -e 0 unrecognized_ext2.o -o unrecognized_ext2 2>&1 | count 0
+# RUN: llvm-readobj --arch-specific unrecognized_ext2 | FileCheck %s --check-prefix=UNRECOGNIZED_EXT2
 
 # RUN: llvm-mc -filetype=obj -triple=riscv64 unrecognized_version.s -o unrecognized_version.o
-# RUN: not ld.lld unrecognized_version.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=UNRECOGNIZED_VERSION --implicit-check-not=error:
-# UNRECOGNIZED_VERSION: error: unrecognized_version.o:(.riscv.attributes): rv64i99p0: unsupported version number 99.0 for extension 'i'
+# RUN: ld.lld -e 0 unrecognized_version.o -o unrecognized_version 2>&1 | count 0
+# RUN: llvm-readobj --arch-specific unrecognized_version | FileCheck %s --check-prefix=UNRECOGNIZED_VERSION
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 merge_version_test_input.s -o merge_version_test_input.o
+# RUN: ld.lld -e 0 unrecognized_version.o merge_version_test_input.o -o out3 2>&1 | count 0
+# RUN: llvm-readobj --arch-specific out3 | FileCheck %s --check-prefix=CHECK3
 
 # RUN: llvm-mc -filetype=obj -triple=riscv64 invalid_arch1.s -o invalid_arch1.o
-# RUN: ld.lld -e 0 invalid_arch1.o -o invalid_arch1
-# RUN: llvm-readobj --arch-specific invalid_arch1 | FileCheck %s --check-prefix=INVALID_ARCH1
+# RUN: not ld.lld invalid_arch1.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=INVALID_ARCH1 --implicit-check-not=error:
+# INVALID_ARCH1: error: invalid_arch1.o:(.riscv.attributes): rv64i2: extension lacks version in expected format
 
 ## A zero value attribute is not printed.
 # RUN: llvm-mc -filetype=obj -triple=riscv64 unaligned_access_0.s -o unaligned_access_0.o
 # RUN: ld.lld -e 0 --fatal-warnings a.o unaligned_access_0.o -o unaligned_access_0
 # RUN: llvm-readobj -A unaligned_access_0 | FileCheck /dev/null --implicit-check-not='TagName: unaligned_access'
 
 ## Differing stack_align values lead to an error.
 # RUN: llvm-mc -filetype=obj -triple=riscv64 diff_stack_align.s -o diff_stack_align.o

 # CHECK2-NEXT:         Tag: 5
 # CHECK2-NEXT:         TagName: arch
 # CHECK2-NEXT:         Value: rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_zkt1p0_zve32f1p0_zve32x1p0_zvl32b1p0
 # CHECK2-NEXT:       }
 # CHECK2-NEXT:     }
 # CHECK2-NEXT:   }
 # CHECK2-NEXT: }
 
+# CHECK3:      BuildAttributes {
+# CHECK3-NEXT:   FormatVersion: 0x41
+# CHECK3-NEXT:   Section 1 {
+# CHECK3-NEXT:     SectionLength: 26
+# CHECK3-NEXT:     Vendor: riscv
+# CHECK3-NEXT:     Tag: Tag_File (0x1)
+# CHECK3-NEXT:     Size: 16
+# CHECK3-NEXT:     FileAttributes {
+# CHECK3-NEXT:       Attribute {
+# CHECK3-NEXT:         Tag: 5
+# CHECK3-NEXT:         TagName: arch
+# CHECK3-NEXT:         Value: rv64i99p0
+# CHECK3-NEXT:       }
+# CHECK3-NEXT:     }
+# CHECK3-NEXT:   }
+# CHECK3-NEXT: }
+
 #--- a.s
 .attribute stack_align, 16
 .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 .attribute unaligned_access, 0
 
 #--- b.s
 .attribute stack_align, 16
 .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 .attribute priv_spec, 2
 .attribute priv_spec_minor, 2
 
 #--- c.s
 .attribute stack_align, 16
 .attribute arch, "rv64i2p0_f2p0_zkt1p0_zve32f1p0_zve32x1p0_zvl32b1p0"
 .attribute unaligned_access, 1
 .attribute priv_spec, 2
 .attribute priv_spec_minor, 2
 
 #--- unrecognized_ext1.s
+# UNRECOGNIZED_EXT1:      BuildAttributes {
+# UNRECOGNIZED_EXT1-NEXT:   FormatVersion: 0x41
+# UNRECOGNIZED_EXT1-NEXT:   Section 1 {
+# UNRECOGNIZED_EXT1-NEXT:     SectionLength: 30
+# UNRECOGNIZED_EXT1-NEXT:     Vendor: riscv
+# UNRECOGNIZED_EXT1-NEXT:     Tag: Tag_File (0x1)
+# UNRECOGNIZED_EXT1-NEXT:     Size: 20
+# UNRECOGNIZED_EXT1-NEXT:     FileAttributes {
+# UNRECOGNIZED_EXT1-NEXT:       Attribute {
+# UNRECOGNIZED_EXT1-NEXT:         Tag: 5
+# UNRECOGNIZED_EXT1-NEXT:         TagName: arch
+# UNRECOGNIZED_EXT1-NEXT:         Value: rv64i2p0_y2p0
+# UNRECOGNIZED_EXT1-NEXT:       }
+# UNRECOGNIZED_EXT1-NEXT:     }
+# UNRECOGNIZED_EXT1-NEXT:   }
+# UNRECOGNIZED_EXT1-NEXT: }
 .section .riscv.attributes,"",@0x70000003
 .byte 0x41
 .long .Lend-.riscv.attributes-1
 .asciz "riscv"  # vendor
 .Lbegin:
 .byte 1  # Tag_File
 .long .Lend-.Lbegin
 .byte 5  # Tag_RISCV_arch
 .asciz "rv64i2p0_y2p0"
 .Lend:
 
 #--- unrecognized_ext2.s
+# UNRECOGNIZED_EXT2:      BuildAttributes {
+# UNRECOGNIZED_EXT2-NEXT:   FormatVersion: 0x41
+# UNRECOGNIZED_EXT2-NEXT:   Section 1 {
+# UNRECOGNIZED_EXT2-NEXT:     SectionLength: 36
+# UNRECOGNIZED_EXT2-NEXT:     Vendor: riscv
+# UNRECOGNIZED_EXT2-NEXT:     Tag: Tag_File (0x1)
+# UNRECOGNIZED_EXT2-NEXT:     Size: 26
+# UNRECOGNIZED_EXT2-NEXT:     FileAttributes {
+# UNRECOGNIZED_EXT2-NEXT:       Attribute {
+# UNRECOGNIZED_EXT2-NEXT:         Tag: 5
+# UNRECOGNIZED_EXT2-NEXT:         TagName: arch
+# UNRECOGNIZED_EXT2-NEXT:         Value: rv64i2p0_zmadeup1p0
+# UNRECOGNIZED_EXT2-NEXT:       }
+# UNRECOGNIZED_EXT2-NEXT:     }
+# UNRECOGNIZED_EXT2-NEXT:   }
+# UNRECOGNIZED_EXT2-NEXT: }
 .section .riscv.attributes,"",@0x70000003
 .byte 0x41
 .long .Lend-.riscv.attributes-1
 .asciz "riscv"  # vendor
 .Lbegin:
 .byte 1  # Tag_File
 .long .Lend-.Lbegin
 .byte 5  # Tag_RISCV_arch
 .asciz "rv64i2p0_zmadeup1p0"
 .Lend:
 
 #--- unrecognized_version.s
+# UNRECOGNIZED_VERSION:      BuildAttributes {
+# UNRECOGNIZED_VERSION-NEXT:   FormatVersion: 0x41
+# UNRECOGNIZED_VERSION-NEXT:   Section 1 {
+# UNRECOGNIZED_VERSION-NEXT:     SectionLength: 26
+# UNRECOGNIZED_VERSION-NEXT:     Vendor: riscv
+# UNRECOGNIZED_VERSION-NEXT:     Tag: Tag_File (0x1)
+# UNRECOGNIZED_VERSION-NEXT:     Size: 16
+# UNRECOGNIZED_VERSION-NEXT:     FileAttributes {
+# UNRECOGNIZED_VERSION-NEXT:       Attribute {
+# UNRECOGNIZED_VERSION-NEXT:         Tag: 5
+# UNRECOGNIZED_VERSION-NEXT:         TagName: arch
+# UNRECOGNIZED_VERSION-NEXT:         Value: rv64i99p0
+# UNRECOGNIZED_VERSION-NEXT:       }
+# UNRECOGNIZED_VERSION-NEXT:     }
+# UNRECOGNIZED_VERSION-NEXT:   }
+# UNRECOGNIZED_VERSION-NEXT: }
 .section .riscv.attributes,"",@0x70000003
 .byte 0x41
 .long .Lend-.riscv.attributes-1
 .asciz "riscv"  # vendor
 .Lbegin:
 .byte 1  # Tag_File
 .long .Lend-.Lbegin
 .byte 5  # Tag_RISCV_arch
 .asciz "rv64i99p0"
 .Lend:
 
+#--- merge_version_test_input.s
+.section .riscv.attributes,"",@0x70000003
+.byte 0x41
+.long .Lend-.riscv.attributes-1
+.asciz "riscv"  # vendor
+.Lbegin:
+.byte 1  # Tag_File
+.long .Lend-.Lbegin
+.byte 5  # Tag_RISCV_arch
+.asciz "rv64i2p1"
+.Lend:
+
 #--- invalid_arch1.s
-# INVALID_ARCH1:      BuildAttributes {
-# INVALID_ARCH1-NEXT:   FormatVersion: 0x41
-# INVALID_ARCH1-NEXT:   Section 1 {
-# INVALID_ARCH1-NEXT:     SectionLength: 25
-# INVALID_ARCH1-NEXT:     Vendor: riscv
-# INVALID_ARCH1-NEXT:     Tag: Tag_File (0x1)
-# INVALID_ARCH1-NEXT:     Size: 15
-# INVALID_ARCH1-NEXT:     FileAttributes {
-# INVALID_ARCH1-NEXT:       Attribute {
-# INVALID_ARCH1-NEXT:         Tag: 5
-# INVALID_ARCH1-NEXT:         TagName: arch
-# INVALID_ARCH1-NEXT:         Value: rv64i2p0
-# INVALID_ARCH1-NEXT:       }
-# INVALID_ARCH1-NEXT:     }
-# INVALID_ARCH1-NEXT:   }
-# INVALID_ARCH1-NEXT: }
 .section .riscv.attributes,"",@0x70000003
 .byte 0x41
 .long .Lend-.riscv.attributes-1
 .asciz "riscv"  # vendor
 .Lbegin:
 .byte 1  # Tag_File
 .long .Lend-.Lbegin
 .byte 5  # Tag_RISCV_arch

llvm/include/llvm/Support/RISCVISAInfo.h

RISCVISAInfo(unsigned XLen, OrderedExtensionMap &Exts)
       : XLen(XLen), FLen(0), MinVLen(0), MaxELen(0), MaxELenFp(0), Exts(Exts) {}
 
   /// Parse RISCV ISA info from arch string.
   static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
   parseArchString(StringRef Arch, bool EnableExperimentalExtension,
                   bool ExperimentalExtensionVersionCheck = true,
                   bool IgnoreUnknown = false);
 
+  /// Parse RISCV ISA info from an arch string that is already in normalized
+  /// form (as defined in the psABI). Unlike parseArchString, this function
+  /// will not error for unrecognized extension names or extension versions.
+  static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
+  parseNormalizedArchString(StringRef Arch);
+
   /// Parse RISCV ISA info from feature vector.
   static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
   parseFeatures(unsigned XLen, const std::vector<std::string> &Features);
 
   /// Convert RISCV ISA info to a feature vector.
   void toFeatures(std::vector<StringRef> &Features,
                   llvm::function_ref<StringRef(const Twine &)> StrAlloc,
                   bool AddAllExtensions) const;

llvm/lib/Support/RISCVISAInfo.cpp

for (auto &Feature : Features) {
     else
       ISAInfo->Exts.erase(ExtName.str());
   }
 
   return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
 }
 
 llvm::Expected<std::unique_ptr<RISCVISAInfo>>
+RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
+  if (llvm::any_of(Arch, isupper)) {
+    return createStringError(errc::invalid_argument,
+                             "string must be lowercase");
+  }
+  // Must start with a valid base ISA name.
+  unsigned XLen;
+  if (Arch.startswith("rv32i") || Arch.startswith("rv32e"))
+    XLen = 32;
+  else if (Arch.startswith("rv64i") || Arch.startswith("rv64e"))
+    XLen = 64;
+  else
+    return createStringError(errc::invalid_argument,
+                             "arch string must begin with valid base ISA");
+  std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
+  // Discard rv32/rv64 prefix.
+  Arch = Arch.substr(4);
+
+  // Each extension is of the form ${name}${major_version}p${minor_version}
+  // and separated by _. Split by _ and then extract the name and version
+  // information for each extension.
+  SmallVector<StringRef, 8> Split;
+  Arch.split(Split, '_');
+  for (StringRef Ext : Split) {
+    StringRef Prefix, MinorVersionStr;
+    std::tie(Prefix, MinorVersionStr) = Ext.rsplit('p');
+    if (MinorVersionStr.empty())
+      return createStringError(errc::invalid_argument,
+                               "extension lacks version in expected format");
+    unsigned MajorVersion, MinorVersion;
+    if (MinorVersionStr.getAsInteger(10, MinorVersion))
+      return createStringError(errc::invalid_argument,
+                               "failed to parse minor version number");
+
+    // Split Prefix into the extension name and the major version number
+    // (the trailing digits of Prefix).
+    int TrailingDigits = 0;
+    StringRef ExtName = Prefix;
+    while (!ExtName.empty()) {
+      if (!isDigit(ExtName.back()))
+        break;
+      ExtName = ExtName.drop_back(1);
+      TrailingDigits++;
+    }
+    if (!TrailingDigits)
+      return createStringError(errc::invalid_argument,
+                               "extension lacks version in expected format");
+
+    StringRef MajorVersionStr = Prefix.take_back(TrailingDigits);
+    if (MajorVersionStr.getAsInteger(10, MajorVersion))
+      return createStringError(errc::invalid_argument,
+                               "failed to parse major version number");
+    ISAInfo->addExtension(ExtName, MajorVersion, MinorVersion);
+  }
+  ISAInfo->updateFLen();
+  ISAInfo->updateMinVLen();
+  ISAInfo->updateMaxELen();
+  return std::move(ISAInfo);
+}
+
+llvm::Expected<std::unique_ptr<RISCVISAInfo>>
 RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
                               bool ExperimentalExtensionVersionCheck,
                               bool IgnoreUnknown) {
   // RISC-V ISA strings must be lowercase.
   if (llvm::any_of(Arch, isupper)) {
     return createStringError(errc::invalid_argument,
                              "string must be lowercase");
   }

llvm/unittests/Support/CMakeLists.txt

add_llvm_unittest(SupportTests
   ParallelTest.cpp
   Path.cpp
   ProcessTest.cpp
   ProgramTest.cpp
   RegexTest.cpp
   ReverseIterationTest.cpp
   ReplaceFileTest.cpp
   RISCVAttributeParserTest.cpp
+  RISCVISAInfoTest.cpp
   ScaledNumberTest.cpp
   ScopedPrinterTest.cpp
   SHA256.cpp
   SourceMgrTest.cpp
   SpecialCaseListTest.cpp
   SuffixTreeTest.cpp
   SwapByteOrderTest.cpp
   TarWriterTest.cpp

llvm/unittests/Support/RISCVISAInfoTest.cpp

+//===-- unittests/RISCVISAInfoTest.cpp ------------------------------------===//
+//
+// 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/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+bool operator==(const llvm::RISCVExtensionInfo &A,
+                const llvm::RISCVExtensionInfo &B) {
+  return A.ExtName == B.ExtName && A.MajorVersion == B.MajorVersion &&
+         A.MinorVersion == B.MinorVersion;
+}
+
+TEST(ParseNormalizedArchString, RejectsUpperCase) {
+  for (StringRef Input : {"RV32", "rV64", "rv32i2P0", "rv64i2p0_A2p0"}) {
+    EXPECT_EQ(
+        toString(RISCVISAInfo::parseNormalizedArchString(Input).takeError()),
+        "string must be lowercase");
+  }
+}
+
+TEST(ParseNormalizedArchString, RejectsInvalidBaseISA) {
+  for (StringRef Input : {"rv32", "rv64", "rv32j", "rv65i"}) {
+    EXPECT_EQ(
+        toString(RISCVISAInfo::parseNormalizedArchString(Input).takeError()),
+        "arch string must begin with valid base ISA");
+  }
+}
+
+TEST(ParseNormalizedArchString, RejectsMalformedInputs) {
+  for (StringRef Input : {"rv64i2p0_", "rv32i2p0__a2p0", "rv32e2.0", "rv64e2p",
+                          "rv32i", "rv64ip1"}) {
+    EXPECT_EQ(
+        toString(RISCVISAInfo::parseNormalizedArchString(Input).takeError()),
+        "extension lacks version in expected format");
+  }
+}
+
+TEST(ParseNormalizedArchString, AcceptsValidBaseISAsAndSetsXLen) {
+  auto MaybeRV32I = RISCVISAInfo::parseNormalizedArchString("rv32i2p0");
+  ASSERT_THAT_EXPECTED(MaybeRV32I, Succeeded());
+  RISCVISAInfo &InfoRV32I = **MaybeRV32I;
+  EXPECT_EQ(InfoRV32I.getExtensions().size(), 1UL);
+  EXPECT_TRUE(InfoRV32I.getExtensions().at("i") ==
+              (RISCVExtensionInfo{"i", 2, 0}));
+  EXPECT_EQ(InfoRV32I.getXLen(), 32U);
+
+  auto MaybeRV32E = RISCVISAInfo::parseNormalizedArchString("rv32e2p0");
+  ASSERT_THAT_EXPECTED(MaybeRV32E, Succeeded());
+  RISCVISAInfo &InfoRV32E = **MaybeRV32E;
+  EXPECT_EQ(InfoRV32E.getExtensions().size(), 1UL);
+  EXPECT_TRUE(InfoRV32E.getExtensions().at("e") ==
+              (RISCVExtensionInfo{"e", 2, 0}));
+  EXPECT_EQ(InfoRV32I.getXLen(), 32U);
+
+  auto MaybeRV64I = RISCVISAInfo::parseNormalizedArchString("rv64i2p0");
+  ASSERT_THAT_EXPECTED(MaybeRV64I, Succeeded());
+  RISCVISAInfo &InfoRV64I = **MaybeRV64I;
+  EXPECT_EQ(InfoRV64I.getExtensions().size(), 1UL);
+  EXPECT_TRUE(InfoRV64I.getExtensions().at("i") ==
+              (RISCVExtensionInfo{"i", 2, 0}));
+  EXPECT_EQ(InfoRV64I.getXLen(), 64U);
+
+  auto MaybeRV64E = RISCVISAInfo::parseNormalizedArchString("rv64e2p0");
+  ASSERT_THAT_EXPECTED(MaybeRV64E, Succeeded());
+  RISCVISAInfo &InfoRV64E = **MaybeRV64E;
+  EXPECT_EQ(InfoRV64E.getExtensions().size(), 1UL);
+  EXPECT_TRUE(InfoRV64E.getExtensions().at("e") ==
+              (RISCVExtensionInfo{"e", 2, 0}));
+  EXPECT_EQ(InfoRV64I.getXLen(), 64U);
+}
+
+TEST(ParseNormalizedArchString, AcceptsArbitraryExtensionsAndVersions) {
+  auto MaybeISAInfo = RISCVISAInfo::parseNormalizedArchString(
+      "rv64i5p1_m3p2_zmadeup11p12_sfoo2p0_xbar3p0");
+  ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+  RISCVISAInfo &Info = **MaybeISAInfo;
+  EXPECT_EQ(Info.getExtensions().size(), 5UL);
+  EXPECT_TRUE(Info.getExtensions().at("i") == (RISCVExtensionInfo{"i", 5, 1}));
+  EXPECT_TRUE(Info.getExtensions().at("m") == (RISCVExtensionInfo{"m", 3, 2}));
+  EXPECT_TRUE(Info.getExtensions().at("zmadeup") ==
+              (RISCVExtensionInfo{"zmadeup", 11, 12}));
+  EXPECT_TRUE(Info.getExtensions().at("sfoo") ==
+              (RISCVExtensionInfo{"sfoo", 2, 0}));
+  EXPECT_TRUE(Info.getExtensions().at("xbar") ==
+              (RISCVExtensionInfo{"xbar", 3, 0}));
+}
+
+TEST(ParseNormalizedArchString, UpdatesFLenMinVLenMaxELen) {
+  auto MaybeISAInfo = RISCVISAInfo::parseNormalizedArchString(
+      "rv64i2p0_d2p0_zvl64b1p0_zve64d1p0");
+  ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+  RISCVISAInfo &Info = **MaybeISAInfo;
+  EXPECT_EQ(Info.getXLen(), 64U);
+  EXPECT_EQ(Info.getFLen(), 64U);
+  EXPECT_EQ(Info.getMinVLen(), 64U);
+  EXPECT_EQ(Info.getMaxELen(), 64U);
+}