[PATCH] D115668: [RISCV] Add a table for extension implications.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:725
+                               [](const ImpliedExtsEntry &Entry,
+                                  StringRef Ext) { return Entry.Name < Ext; });
+    if (I != std::end(ImpliedExts) && I->Name == Ext.first) {
----------------
eopXD wrote:
> Does this mean the `ImpliedExts` have to have extensions sorted in lexicological order? If so adding the `zvl` extension would look like: (I think it looks strange in my opinion, but no big problem though
> 
> ```
> static constexpr ImpliedExtsEntry ImpliedExts[] = {
>     {"v", ImpliedExtsV},
>     {"zfh", ImpliedExtsZfh},
>     {"zvl64b", ImpliedExtsZvl64b},
>     {"zvl1024b", ImpliedExtsZvl1024b},
>     {"zvl128b", ImpliedExtsZvl128b},
>     {"zvl16384b", ImpliedExtsZvl16384b},
>     {"zvl2048b", ImpliedExtsZvl2048b},
>     {"zvl256b", ImpliedExtsZvl256b},
>     {"zvl32768b", ImpliedExtsZvl32768b},
>     {"zvl4096b", ImpliedExtsZvl4096b},
>     {"zvl512b", ImpliedExtsZvl512b},
>     {"zvl65536b", ImpliedExtsZvl65536b},
>     {"zvl8192b", ImpliedExtsZvl8192b},
> };
> ```
> 
Yes. It is unfortunate that they have to be ordered like that. I should also add a check to verify the table is sorted to prevent mistakes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115668/new/

https://reviews.llvm.org/D115668