[PATCH] D156084: [RISCV] Update Zvk shorthand extension to 1.0.0-rc1

Mon Jul 24 01:24:42 PDT 2023

Jim added inline comments.

================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:604
+                       "other extensions: Zvkned, Zvknhb, Zvbb and Zvkt.",
+                       [FeatureStdExtZvkned, FeatureStdExtZvknhb,
+                        FeatureStdExtZvbb, FeatureStdExtZvkt]>;
----------------
ego wrote:
> A few questions to improve my understanding. I am confused about the interactions between RISCVFeatures.td and RISCVISAInfo.cpp.
> 
> Prior to this commit, we did not list the implied extensions as part of the shorthand extensions. An expansion occurred during ISA string processing in llvm/lib/Support/RISCVISAInfo.cpp. Was this expansion missing in some cases, or are we now duplicating the expansion logic?
> 
> In RISCVISAInfo.cpp, there is also logic (see CombineIntoExts) to reveal the existence of those shorthand extensions when every component sub-extension has been declared. Is similar logic in place for those FeatureStdExt...?
I am also not sure should we add the impiled extension as part of the shorthand extension in RISCVFeatures.td.
That I saw `FeatureStdExtZkn` and `FeatureStdExtZks` has listed the impiled extensions.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156084