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

Reply via email to