eopXD added inline comments.
================ Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:24 + unsigned Minor; + bool operator==(const RISCVExtensionVersion &Vers) const { + return this->Major == Vers.Major && this->Minor == Vers.Minor; ---------------- ym1813382441 wrote: > eopXD wrote: > > Does the structure already work as-is? > I've tested it and it works. 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. ================ Comment at: llvm/lib/Support/RISCVISAInfo.cpp:190 + RISCVExtensionVersion Version) { + assert(!Exts.count(ExtName.str()) && "Extension already exists"); + Exts[ExtName.str()] = Version; ---------------- ym1813382441 wrote: > eopXD wrote: > > Maybe handling this with a compiler error is better than an assertion error. > Currently the compiler does not allow the same extension to appear more than > once. > If it happens something has gone wrong. Yes. So we need to have an error instead of an assertion error to do this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144696/new/ https://reviews.llvm.org/D144696 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits