[RISCV][NFC] Package version number information using RISCVExtensionVersion.

[ym1813382441]

Summary

This helps to support multiple version number extensions later.

[eopXD]

Thank you for the patch. May you explain how this would help support multi-versioning?

llvm/include/llvm/Support/RISCVISAInfo.h

24

Does the structure already work as-is?

llvm/lib/Support/RISCVISAInfo.cpp

190

Maybe handling this with a compiler error is better than an assertion error.

[ym1813382441]

I want to use arrays to maintain the supported version number of each extension, this patch in using the find algorithm helps to simplify the code.

llvm/include/llvm/Support/RISCVISAInfo.h

24

I've tested it and it works.

llvm/lib/Support/RISCVISAInfo.cpp

190

Currently the compiler does not allow the same extension to appear more than once.
If it happens something has gone wrong.

[craig.topper]

Can you outline your full plan and why you want to do this. We need to see the bigger picture.

[ym1813382441]

[ym1813382441]

In D144696#4149516, @craig.topper wrote:

Can you outline your full plan and why you want to do this. We need to see the bigger picture.

In D144696#4149545, @ym1813382441 wrote:

I'm implementing this idea

[craig.topper]

In D144696#4149545, @ym1813382441 wrote:

I meant what extensions are you planning to support multiple versions for and why? How will this work in the assembler, code generator, and disassembler which use SubtargetFeatures not RISCVISAInfo.

[ym1813382441]

In D144696#4149560, @craig.topper wrote:

In D144696#4149545, @ym1813382441 wrote:

I meant what extensions are you planning to support multiple versions for and why? How will this work in the assembler, code generator, and disassembler which use SubtargetFeatures not RISCVISAInfo.

My intention is to support RVV0.71, since specific cpu's exist for the XuanTie C900 series. In LLVM use Feature to distinguish between different versions of extensions. I am considering how to solve this problem, using multiple .td files in the worst case.

[• pcwang-thead]

My intention is to support RVV0.71, since specific cpu's exist for the XuanTie C900 series. In LLVM use Feature to distinguish between different versions of extensions. I am considering how to solve this problem, using multiple .td files in the worst case.

@ym1813382441
Thanks for your works and we really appreciate it. :-)
Similar proposal has been rejected by the community (please refer to D115921 for previous discussion) and I don't know if others have changed their minds.

[craig.topper]

How much of the existing vector code in the backend including tablegen can be reused for 0.71? How much of the intrinsic interface can be used? If we’re talking about completely duplicating everything it is very unlikely we can accept it. RISCVGenDAGISel.inc, for example, is already significantly larger than any other target in tree. I don’t know how big it can get before it becomes an issue.

How many people are going to maintain this other version?

[asb]

@ym1813382441 thanks for the contribution. In many cases, this kind of incremental and tightly focused patch is a good way to start things. But in this case, there's a few bigger issues.

• As has been pointed out, the discussion about supporting multiple ISA versions has been started before. The barrier isn't the code change required, but more about agreeing we actually want/need to support multiple extension versions simultaneously (RISC-V International seem to have indicated they'd add new named extensions rather than add instructions to an existing extension in the future), and perhaps the biggest blocker has been around the handling of build attributes, what is accepted / produces an error / produces a warning by what tool and so on. We've been discussing this in the recent RISC-V LLVM sync-up calls and a few people who've been thinking about these issues for a while are hoping to come up with a proposal in the coming weeks.
• If the ultimate intention is to support V0.7, then as Craig suggests that's a much bigger discussion. We really need to understand what the level of changes would be required, how this would impact in-tree code for V1.0, what the burden might be going forwards and so on. Not all points are relevant, but I've often thought that the considerations for Contributing Extensions to Clang provide a good starting point. e.g.
  • Evidence of a significant user community
  • A specific need to reside within the upstream tree
  • A long term support plan
  • A high quality implementation
  • Testing

[ym1813382441]

In D144696#4149706, @craig.topper wrote:

How much of the existing vector code in the backend including tablegen can be reused for 0.71? How much of the intrinsic interface can be used? If we’re talking about completely duplicating everything it is very unlikely we can accept it. RISCVGenDAGISel.inc, for example, is already significantly larger than any other target in tree. I don’t know how big it can get before it becomes an issue.

How many people are going to maintain this other version?

I will not submit RVV0.71 to the community, I just want to extend the interface to support multiple versions, this will help to solve the compatibility issue of different versions of the extension, this issue is common.

[• pcwang-thead]

In D144696#4149706, @craig.topper wrote:

How much of the existing vector code in the backend including tablegen can be reused for 0.71? How much of the intrinsic interface can be used? If we’re talking about completely duplicating everything it is very unlikely we can accept it. RISCVGenDAGISel.inc, for example, is already significantly larger than any other target in tree. I don’t know how big it can get before it becomes an issue.

How many people are going to maintain this other version?

Just FYI. We have tried our best to reuse code for RVV 1.0 and there are about 500 lines for C intrinsics, 80 lines for LLVM intrinsics, 1200 lines for pseudos and patterns and 1500 lines for custom ISel. The pseudos/patterns/ISel parts can be less since we have duplicated some code and the impact on RISCVGenDAGISel.inc is hard to estimate because we have some custom extensions. And for MC layer, the code are completely duplicated since a lot of instructions have different encodings.

[eopXD]

llvm/include/llvm/Support/RISCVISAInfo.h

24

I mean, without your modification to the structure, the original structure already works. So I don't see how refactoring here can help you more on supporting multiple versions of the same extension.

llvm/lib/Support/RISCVISAInfo.cpp

190

Yes. So we need to have an error instead of an assertion error to do this.

[ym1813382441]

llvm/include/llvm/Support/RISCVISAInfo.h

24

I just move RISCVExtensionVersion structure here, and remove RISCVExtensionInfo structure.
I think that RISCVExtensionInfo is redundant.

llvm/lib/Support/RISCVISAInfo.cpp

190

Are you expecting an error message from the compiler?

[ym1813382441]

RISC-V International seem to have indicated they'd add new named extensions rather than add instructions to an existing extension in the future.
Related discussions at D115921

clang/lib/Basic/Targets/RISCV.cpp

void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
 
   if (ABIName == "ilp32e")
     Builder.defineMacro("__riscv_abi_rve");
 
   Builder.defineMacro("__riscv_arch_test");
 
   for (auto &Extension : ISAInfo->getExtensions()) {
     auto ExtName = Extension.first;
-    auto ExtInfo = Extension.second;
+    auto Version = Extension.second;
 
     Builder.defineMacro(
         Twine("__riscv_", ExtName),
-        Twine(getVersionValue(ExtInfo.MajorVersion, ExtInfo.MinorVersion)));
+        Twine(getVersionValue(Version.Major, Version.Minor)));
   }
 
   if (ISAInfo->hasExtension("m") || ISAInfo->hasExtension("zmmul"))
     Builder.defineMacro("__riscv_mul");
 
   if (ISAInfo->hasExtension("m")) {
     Builder.defineMacro("__riscv_div");
     Builder.defineMacro("__riscv_muldiv");

llvm/include/llvm/Support/RISCVISAInfo.h

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
 namespace llvm {
-struct RISCVExtensionInfo {
-  std::string ExtName;
-  unsigned MajorVersion;
-  unsigned MinorVersion;
+/// Represents the major and version number components of a RISC-V extension
+struct RISCVExtensionVersion {
+  unsigned Major;
+  unsigned Minor;
+  bool operator==(const RISCVExtensionVersion &Vers) const {
+    return this->Major == Vers.Major && this->Minor == Vers.Minor;
+  }
+  bool operator!=(const RISCVExtensionVersion &Vers) const {
+    return !(*this == Vers);
+  }
 };
 
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
   RISCVISAInfo &operator=(const RISCVISAInfo &) = delete;
 
   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 std::map, it's specialized to keep entries
   /// in canonical order of extension.
-  typedef std::map<std::string, RISCVExtensionInfo, ExtensionComparator>
+  typedef std::map<std::string, RISCVExtensionVersion, ExtensionComparator>
       OrderedExtensionMap;
 
   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,
public:
 
   bool hasExtension(StringRef Ext) const;
   std::string toString() const;
   std::vector<std::string> toFeatureVector() const;
   StringRef computeDefaultABI() const;
 
   static bool isSupportedExtensionFeature(StringRef Ext);
   static bool isSupportedExtension(StringRef Ext);
-  static bool isSupportedExtension(StringRef Ext, unsigned MajorVersion,
-                                   unsigned MinorVersion);
+  static bool isSupportedExtension(StringRef Ext,
+                                   RISCVExtensionVersion Version);
   static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
   postProcessAndChecking(std::unique_ptr<RISCVISAInfo> &&ISAInfo);
 
 private:
   RISCVISAInfo(unsigned XLen)
       : XLen(XLen), FLen(0), MinVLen(0), MaxELen(0), MaxELenFp(0) {}
 
   unsigned XLen;
   unsigned FLen;
   unsigned MinVLen;
   unsigned MaxELen, MaxELenFp;
 
   OrderedExtensionMap Exts;
 
-  void addExtension(StringRef ExtName, unsigned MajorVersion,
-                    unsigned MinorVersion);
+  void addExtension(StringRef ExtName, RISCVExtensionVersion Version);
 
   Error checkDependency();
 
   void updateImplication();
   void updateCombination();
   void updateFLen();
   void updateMinVLen();
   void updateMaxELen();
 };
 
 } // namespace llvm
 
 #endif

llvm/lib/Support/RISCVISAInfo.cpp

 #include <array>
 #include <optional>
 #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 RISCVSupportedExtension {
   const char *Name;
   /// Supported version.
   RISCVExtensionVersion Version;
 };
 
 } // end anonymous namespace
for (auto &ExtInfo : {ArrayRef(SupportedExtensions),
     if (ExtensionInfoIterator == ExtInfo.end()) {
       continue;
     }
     return ExtensionInfoIterator->Version;
   }
   return std::nullopt;
 }
 
-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;
+void RISCVISAInfo::addExtension(StringRef ExtName,
+                                RISCVExtensionVersion Version) {
+  assert(!Exts.count(ExtName.str()) && "Extension already exists");
+  Exts[ExtName.str()] = Version;
 }
 
 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"))
else
     return llvm::any_of(SupportedExtensions, FindByName(Ext));
 }
 
 bool RISCVISAInfo::isSupportedExtension(StringRef Ext) {
   return llvm::any_of(SupportedExtensions, FindByName(Ext)) ||
          llvm::any_of(SupportedExperimentalExtensions, FindByName(Ext));
 }
 
-bool RISCVISAInfo::isSupportedExtension(StringRef Ext, unsigned MajorVersion,
-                                        unsigned MinorVersion) {
+bool RISCVISAInfo::isSupportedExtension(StringRef Ext,
+                                        RISCVExtensionVersion Version) {
   auto FindByNameAndVersion = [=](const RISCVSupportedExtension &ExtInfo) {
-    return ExtInfo.Name == Ext && (MajorVersion == ExtInfo.Version.Major) &&
-           (MinorVersion == ExtInfo.Version.Minor);
+    return ExtInfo.Name == Ext && (Version == ExtInfo.Version);
   };
   return llvm::any_of(SupportedExtensions, FindByNameAndVersion) ||
          llvm::any_of(SupportedExperimentalExtensions, FindByNameAndVersion);
 }
 
 bool RISCVISAInfo::hasExtension(StringRef Ext) const {
   stripExperimentalPrefix(Ext);
 
void RISCVISAInfo::toFeatures(
   }
 }
 
 // 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 Ext, StringRef In, unsigned &Major,
-                                 unsigned &Minor, unsigned &ConsumeLength,
+static Error getExtensionVersion(StringRef Ext, StringRef In,
+                                 RISCVExtensionVersion &Version,
+                                 unsigned &ConsumeLength,
                                  bool EnableExperimentalExtension,
                                  bool ExperimentalExtensionVersionCheck) {
   StringRef MajorStr, MinorStr;
-  Major = 0;
-  Minor = 0;
+  Version.Major = 0;
+  Version.Minor = 0;
   ConsumeLength = 0;
   MajorStr = In.take_while(isDigit);
   In = In.substr(MajorStr.size());
 
   if (!MajorStr.empty() && In.consume_front("p")) {
     MinorStr = In.take_while(isDigit);
     In = In.substr(MajorStr.size() + MinorStr.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 '" + Ext + "'");
     }
   }
 
-  if (!MajorStr.empty() && MajorStr.getAsInteger(10, Major))
+  if (!MajorStr.empty() && MajorStr.getAsInteger(10, Version.Major))
     return createStringError(
         errc::invalid_argument,
         "Failed to parse major version number for extension '" + Ext + "'");
 
-  if (!MinorStr.empty() && MinorStr.getAsInteger(10, Minor))
+  if (!MinorStr.empty() && MinorStr.getAsInteger(10, Version.Minor))
     return createStringError(
         errc::invalid_argument,
         "Failed to parse minor version number for extension '" + Ext + "'");
 
   ConsumeLength = MajorStr.size();
 
   if (!MinorStr.empty())
     ConsumeLength += MinorStr.size() + 1 /*'p'*/;
if (ExperimentalExtensionVersionCheck &&
         (MajorStr.empty() && MinorStr.empty())) {
       std::string Error =
           "experimental extension requires explicit version number `" +
           Ext.str() + "`";
       return createStringError(errc::invalid_argument, Error);
     }
 
     auto SupportedVers = *ExperimentalExtension;
-    if (ExperimentalExtensionVersionCheck &&
-        (Major != SupportedVers.Major || Minor != SupportedVers.Minor)) {
+    if (ExperimentalExtensionVersionCheck && (Version != SupportedVers)) {
       std::string Error = "unsupported version number " + MajorStr.str();
       if (!MinorStr.empty())
         Error += "." + MinorStr.str();
       Error += " for experimental extension '" + Ext.str() +
                "' (this compiler supports " + utostr(SupportedVers.Major) +
                "." + utostr(SupportedVers.Minor) + ")";
       return createStringError(errc::invalid_argument, Error);
     }
     return Error::success();
   }
 
   // Exception rule for `g`, we don't have clear version scheme for that on
   // ISA spec.
   if (Ext == "g")
     return Error::success();
 
   if (MajorStr.empty() && MinorStr.empty()) {
     if (auto DefaultVersion = findDefaultVersion(Ext)) {
-      Major = DefaultVersion->Major;
-      Minor = DefaultVersion->Minor;
+      Version = *DefaultVersion;
     }
     // No matter found or not, return success, assume other place will
     // verify.
     return Error::success();
   }
 
-  if (RISCVISAInfo::isSupportedExtension(Ext, Major, Minor))
+  if (RISCVISAInfo::isSupportedExtension(Ext, Version))
     return Error::success();
 
   std::string Error = "unsupported version number " + std::string(MajorStr);
   if (!MinorStr.empty())
     Error += "." + MinorStr.str();
   Error += " for extension '" + Ext.str() + "'";
   return createStringError(errc::invalid_argument, Error);
 }
auto ExtensionInfoIterator =
         llvm::find_if(ExtensionInfos, FindByName(ExtName));
 
     // Not all features is related to ISA extension, like `relax` or
     // `save-restore`, skip those feature.
     if (ExtensionInfoIterator == ExtensionInfos.end())
       continue;
 
     if (Add)
-      ISAInfo->addExtension(ExtName, ExtensionInfoIterator->Version.Major,
-                            ExtensionInfoIterator->Version.Minor);
+      ISAInfo->addExtension(ExtName, ExtensionInfoIterator->Version);
     else
       ISAInfo->Exts.erase(ExtName.str());
   }
 
   return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
 }
 
 llvm::Expected<std::unique_ptr<RISCVISAInfo>>
RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
   // Find the very first occurrence of 's', 'x' or 'z'.
   StringRef OtherExts;
   size_t Pos = Exts.find_first_of("zsx");
   if (Pos != StringRef::npos) {
     OtherExts = Exts.substr(Pos);
     Exts = Exts.substr(0, Pos);
   }
 
-  unsigned Major, Minor, ConsumeLength;
-  if (auto E = getExtensionVersion(std::string(1, Baseline), Exts, Major, Minor,
+  RISCVExtensionVersion Version;
+  unsigned ConsumeLength;
+  if (auto E = getExtensionVersion(std::string(1, Baseline), Exts, Version,
                                    ConsumeLength, EnableExperimentalExtension,
                                    ExperimentalExtensionVersionCheck))
     return std::move(E);
 
   if (Baseline == 'g') {
     // No matter which version is given to `g`, we always set imafd to default
     // version since the we don't have clear version scheme for that on
     // ISA spec.
     for (const auto *Ext : {"i", "m", "a", "f", "d"})
-      if (auto Version = findDefaultVersion(Ext))
-        ISAInfo->addExtension(Ext, Version->Major, Version->Minor);
+      if (auto Vers = findDefaultVersion(Ext))
+        ISAInfo->addExtension(Ext, *Vers);
       else
         llvm_unreachable("Default extension version not found?");
   } else
     // Baseline is `i` or `e`
-    ISAInfo->addExtension(std::string(1, Baseline), Major, Minor);
+    ISAInfo->addExtension(std::string(1, Baseline), Version);
 
   // Consume the base ISA version number and any '_' between rvxxx and the
   // first extension
   Exts = Exts.drop_front(ConsumeLength);
   Exts.consume_front("_");
 
   // TODO: Use version number when setting target features
 
if (StdExtsItr == StdExtsEnd) {
       return createStringError(errc::invalid_argument,
                                "invalid standard user-level extension '%c'", C);
     }
 
     // Move to next char to prevent repeated letter.
     ++StdExtsItr;
 
     std::string Next;
-    unsigned Major, Minor, ConsumeLength;
+    RISCVExtensionVersion Version;
+    unsigned ConsumeLength;
     if (std::next(I) != E)
       Next = std::string(std::next(I), E);
-    if (auto E = getExtensionVersion(std::string(1, C), Next, Major, Minor,
+    if (auto E = getExtensionVersion(std::string(1, C), Next, Version,
                                      ConsumeLength, EnableExperimentalExtension,
                                      ExperimentalExtensionVersionCheck)) {
       if (IgnoreUnknown) {
         consumeError(std::move(E));
         GoToNextExt(I, ConsumeLength);
         continue;
       }
       return std::move(E);
     }
 
     // The order is OK, then push it into features.
     // TODO: Use version number when setting target features
     // Currently LLVM supports only "mafdcvh".
     if (!isSupportedExtension(StringRef(&C, 1))) {
       if (IgnoreUnknown) {
         GoToNextExt(I, ConsumeLength);
         continue;
       }
       return createStringError(errc::invalid_argument,
                                "unsupported standard user-level extension '%c'",
                                C);
     }
-    ISAInfo->addExtension(std::string(1, C), Major, Minor);
+    ISAInfo->addExtension(std::string(1, C), Version);
 
     // Consume full extension name and version, including any optional '_'
     // between this extension and the next
     GoToNextExt(I, ConsumeLength);
   }
 
   // Handle other types of extensions other than the standard
   // general purpose and standard user-level extensions.
for (StringRef Ext : Split) {
       }
 
       if (!IgnoreUnknown && Name.size() == Type.size()) {
         return createStringError(errc::invalid_argument,
                                  "%s name missing after '%s'",
                                  Desc.str().c_str(), Type.str().c_str());
       }
 
-      unsigned Major, Minor, ConsumeLength;
-      if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength,
+      RISCVExtensionVersion Version;
+      unsigned ConsumeLength;
+      if (auto E = getExtensionVersion(Name, Vers, Version, ConsumeLength,
                                        EnableExperimentalExtension,
                                        ExperimentalExtensionVersionCheck)) {
         if (IgnoreUnknown) {
           consumeError(std::move(E));
           continue;
         }
         return std::move(E);
       }
 
       // Check if duplicated extension.
       if (!IgnoreUnknown && llvm::is_contained(AllExts, Name)) {
         return createStringError(errc::invalid_argument, "duplicated %s '%s'",
                                  Desc.str().c_str(), Name.str().c_str());
       }
 
-      ISAInfo->addExtension(Name, Major, Minor);
+      ISAInfo->addExtension(Name, Version);
       // Extension format is correct, keep parsing the extensions.
       // TODO: Save Type, Name, Major, Minor to avoid parsing them later.
       AllExts.push_back(Name);
     }
   }
 
   for (auto Ext : AllExts) {
     if (!isSupportedExtension(Ext)) {

 void RISCVISAInfo::updateImplication() {
   bool HasE = Exts.count("e") != 0;
   bool HasI = Exts.count("i") != 0;
 
   // If not in e extension and i extension does not exist, i extension is
   // implied
   if (!HasE && !HasI) {
     auto Version = findDefaultVersion("i");
-    addExtension("i", Version->Major, Version->Minor);
+    addExtension("i", *Version);
   }
 
   assert(llvm::is_sorted(ImpliedExts) && "Table not sorted by Name");
 
   // This loop may execute over 1 iteration since implication can be layered
   // Exits loop if no more implication is applied
   SmallSetVector<StringRef, 16> WorkList;
   for (auto const &Ext : Exts)
     WorkList.insert(Ext.first);
 
   while (!WorkList.empty()) {
     StringRef ExtName = WorkList.pop_back_val();
     auto I = llvm::lower_bound(ImpliedExts, ExtName);
     if (I != std::end(ImpliedExts) && I->Name == ExtName) {
       for (const char *ImpliedExt : I->Exts) {
         if (WorkList.count(ImpliedExt))
           continue;
         if (Exts.count(ImpliedExt))
           continue;
         auto Version = findDefaultVersion(ImpliedExt);
-        addExtension(ImpliedExt, Version->Major, Version->Minor);
+        addExtension(ImpliedExt, *Version);
         WorkList.insert(ImpliedExt);
       }
     }
   }
 }
 
 struct CombinedExtsEntry {
   StringLiteral CombineExt;
for (CombinedExtsEntry CombineIntoExt : CombineIntoExts) {
       auto RequiredExts = CombineIntoExt.RequiredExts;
       if (hasExtension(CombineExt))
         continue;
       bool IsAllRequiredFeatureExist = true;
       for (const char *Ext : RequiredExts)
         IsAllRequiredFeatureExist &= hasExtension(Ext);
       if (IsAllRequiredFeatureExist) {
         auto Version = findDefaultVersion(CombineExt);
-        addExtension(CombineExt, Version->Major, Version->Minor);
+        addExtension(CombineExt, *Version);
         IsNewCombine = true;
       }
     }
   } while (IsNewCombine);
 }
 
 void RISCVISAInfo::updateFLen() {
   FLen = 0;
std::string RISCVISAInfo::toString() const {
   std::string Buffer;
   raw_string_ostream Arch(Buffer);
 
   Arch << "rv" << XLen;
 
   ListSeparator LS("_");
   for (auto const &Ext : Exts) {
     StringRef ExtName = Ext.first;
-    auto ExtInfo = Ext.second;
+    auto Version = Ext.second;
     Arch << LS << ExtName;
-    Arch << ExtInfo.MajorVersion << "p" << ExtInfo.MinorVersion;
+    Arch << Version.Major << "p" << Version.Minor;
   }
 
   return Arch.str();
 }
 
 std::vector<std::string> RISCVISAInfo::toFeatureVector() const {
   std::vector<std::string> FeatureVector;
   for (auto const &Ext : Exts) {