ym1813382441 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;
----------------
eopXD wrote:
> 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.
I just move RISCVExtensionVersion structure here, and remove RISCVExtensionInfo
structure.
I think that RISCVExtensionInfo is redundant.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:190
+ RISCVExtensionVersion Version) {
+ assert(!Exts.count(ExtName.str()) && "Extension already exists");
+ Exts[ExtName.str()] = Version;
----------------
eopXD wrote:
> 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.
Are you expecting an error message from the compiler?
{F26666038 size=full}
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
