[PATCH] D108694: [RISCV] Add the zvl extension according to the v1.0 spec

Yueh-Ting Chen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 29 01:35:30 PST 2021


eopXD added inline comments.


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:741-751
+    {{"zvl1024b"}, {ImpliedExtsZvl1024b}},
+    {{"zvl128b"}, {ImpliedExtsZvl128b}},
+    {{"zvl16384b"}, {ImpliedExtsZvl16384b}},
+    {{"zvl2048b"}, {ImpliedExtsZvl2048b}},
+    {{"zvl256b"}, {ImpliedExtsZvl256b}},
+    {{"zvl32768b"}, {ImpliedExtsZvl32768b}},
+    {{"zvl4096b"}, {ImpliedExtsZvl4096b}},
----------------
achieveartificialintelligence wrote:
> Can we compress the code?
Hi,

Thank you for leaving a comment. Do you mean to embed the list of implied extensions into the declaration of `ImpliedExts`? Like:

```
static constexpr ImpliedExtsEntry ImpliedExts[] = {
    {{"v"}, {{"zvlsseg", "zvl128b"}}},
   ...
```

In my opinion I think compressing here may not help because the indirection is intended to have the implications be in more sorted order that is more human readable since `ImpliedExts` are required to be in lexicographical order.

I am just stating my opinion on my current implementation since I think this is a coding style problem. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108694



More information about the cfe-commits mailing list