[clang] [clang] Enable descriptions for --print-supported-extensions (PR #66715)

Balint Cristian via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 19 02:59:10 PDT 2023


================
@@ -210,24 +210,33 @@ static void verifyTables() {
 #endif
 }
 
-void llvm::riscvExtensionsHelp() {
+void llvm::riscvExtensionsHelp(std::map<StringRef, StringRef> llvmDescMap) {
+
   outs() << "All available -march extensions for RISC-V\n\n";
-  outs() << '\t' << left_justify("Name", 20) << "Version\n";
+  outs() << '\t' << left_justify("Name", 20) << "Version";
+  outs() << (llvmDescMap.empty() ? "\n" : "\tDescription\n");
 
   RISCVISAInfo::OrderedExtensionMap ExtMap;
   for (const auto &E : SupportedExtensions)
     ExtMap[E.Name] = {E.Version.Major, E.Version.Minor};
-  for (const auto &E : ExtMap)
-    outs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+  for (const auto &E : ExtMap) {
+    outs() << format("\t%-20s%d.%d", E.first.c_str(), E.second.MajorVersion,
                      E.second.MinorVersion);
+    outs() << (llvmDescMap.empty() ? "\n"
----------------
cbalint13 wrote:

> In general I don't think we should assume that all named frontend features will be in the backend by the same name, and then also have a description.

Yes, there are misses already, very few ones (less then five for all the 3 arches).
But this will encourage us to a better house-keeping in tablegen (backend) vs. here (frontend).

> Practically, that means I'd prefer we do a lookup for each name, expecting that it might fail. Instead of checking if the map is empty and if not doing an unconditional lookup.

The ```llvmDescMap.empty()``` is for other reason, I was thinking to make the whole ```Description``` column optional. It is useful for unit-test to bypass the whole target-machine creation saga and just look at the essential mandatory two columns (without descriptions column in test-mode).

* See in ```llvm/unittests/Support/RISCVISAInfoTest.cpp```
```
  std::map<StringRef, StringRef> EmptyMap;
  llvm::riscvExtensionsHelp(EmptyMap);
```
Basically with this feature we have an option to skip "Description" column at all.


> In 99.9% of cases, the name probably matches. Maybe even has to match, but I know we've bent that rule in AArch64 in the past, and it wouldn't cost that much in terms of cpu time to do it that way. This is not a hot path, and we're spending time building a map already.

The features amount per arch is less than a hundred, this is a user-help-menu anyway guiding the user.






https://github.com/llvm/llvm-project/pull/66715


More information about the cfe-commits mailing list