[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 522411.
4vtomat added a comment.

Resolved MaskRay's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,6 +139,29 @@
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
+void llvm::riscvExtensionsHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto  : SupportedExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto  : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef ) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -23,6 +23,8 @@
   unsigned MinorVersion;
 };
 
+void riscvExtensionsHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,14 @@
   return 0;
 }
 
+/// Print supported extensions of the RISCV target.
+static int print_supported_extensions() {
+  llvm::riscvExtensionsHelp();
+
+  return 0;
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -223,6 +232,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return print_supported_extensions();
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,94 @@
+// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// CHECK-NOT: warning: argument unused during compilation
+// CHECK-RISCV: Target: riscv64
+// CHECK-NEXT: All available -march extensions for RISC-V
+// CHECK-NEXT: 	NameVersion
+// CHECK-NEXT: 	i   2.0
+// CHECK-NEXT: 	e   1.9
+// CHECK-NEXT: 	m   2.0
+// CHECK-NEXT: 	a   2.0
+// CHECK-NEXT: 	f   2.0
+// CHECK-NEXT: 	d   2.0
+// CHECK-NEXT: 	c   2.0
+// CHECK-NEXT: 	v   1.0
+// CHECK-NEXT: 	h   1.0
+// CHECK-NEXT: 	svinval 1.0
+// CHECK-NEXT: 	svnapot 1.0
+// CHECK-NEXT: 	svpbmt  1.0
+// CHECK-NEXT: 	zicbom  1.0
+// CHECK-NEXT: 	zicbop  1.0
+// CHECK-NEXT: 	zicboz  1.0
+// CHECK-NEXT: 	zicsr   2.0
+// CHECK-NEXT: 	zifencei2.0
+// CHECK-NEXT: 	zihintpause 2.0
+// CHECK-NEXT: 	zmmul   1.0
+// CHECK-NEXT: 	zawrs   1.0
+// CHECK-NEXT: 	zfh 1.0
+// CHECK-NEXT: 	zfhmin  1.0
+// CHECK-NEXT: 	zfinx  

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D146054#4337866 , @kito-cheng 
wrote:

> GCC ins't implement yet, but planed, so add it later I think?
>
> @4vtomat already drop -march=help, @MaskRay did you mind take a look again?

Done. But this update still doesn't look quite polished. If `-march=help` is 
dropped, at the very least the subject needs to updated.




Comment at: clang/tools/driver/cc1_main.cpp:187
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();

Use camelCase for new function names. `StringRef TargetStr`?



Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:26
 
+void riscvMarchHelp();
+

This naming does not match the `--print-supported-extensions` option name.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:143
+void llvm::riscvMarchHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";

For most `--print-*` options, the output goes to stdout instead of stderr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

GCC ins't implement yet, but planed, so add it later I think?

@4vtomat already drop -march=help, @MaskRay did you mind take a look again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 520904.
4vtomat added a comment.

Remove -march=help alias.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,6 +139,29 @@
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
+void llvm::riscvMarchHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto  : SupportedExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto  : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef ) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -23,6 +23,8 @@
   unsigned MinorVersion;
 };
 
+void riscvMarchHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,14 @@
   return 0;
 }
 
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();
+
+  return 0;
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -223,6 +232,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return PrintSupportedExtensions(Clang->getTargetOpts().Triple);
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,94 @@
+// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// CHECK-NOT: warning: argument unused during compilation
+// CHECK-RISCV: Target: riscv64
+// CHECK-NEXT: All available -march extensions for RISC-V
+// CHECK-NEXT: 	NameVersion
+// CHECK-NEXT: 	i   2.0
+// CHECK-NEXT: 	e   1.9
+// CHECK-NEXT: 	m   2.0
+// CHECK-NEXT: 	a   2.0
+// CHECK-NEXT: 	f   2.0
+// CHECK-NEXT: 	d   2.0
+// CHECK-NEXT: 	c   2.0
+// CHECK-NEXT: 	v   1.0
+// CHECK-NEXT: 	h   1.0
+// CHECK-NEXT: 	svinval 1.0
+// CHECK-NEXT: 	svnapot 1.0
+// CHECK-NEXT: 	svpbmt  1.0
+// CHECK-NEXT: 	zicbom  1.0
+// CHECK-NEXT: 	zicbop  1.0
+// CHECK-NEXT: 	zicboz  1.0
+// CHECK-NEXT: 	zicsr   2.0
+// CHECK-NEXT: 	zifencei2.0
+// CHECK-NEXT: 	zihintpause 2.0
+// CHECK-NEXT: 	zmmul   1.0
+// CHECK-NEXT: 	zawrs   1.0
+// CHECK-NEXT: 	zfh 1.0
+// CHECK-NEXT: 	zfhmin  1.0

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`-march=` `-mcpu=` `-mtune=` are from GCC and they traditionally have many 
opinions on the features.
If GCC is going to have `-march=help`, I think adding `-march=help` to Clang to 
match is fine.
But I'd really like to avoid aliases for non-compatibility reasons. They just 
cause confusion and make users undecided about which one is canonical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D146054#4314766 , @kito-cheng 
wrote:

> LGTM, consider about the GNU compatibility, I would that has -march=help form 
> for that.

Are you saying gcc supports march=help? Is that recent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng accepted this revision.
kito-cheng added a comment.

LGTM, consider about the GNU compatibility, I would that has -march=help form 
for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:110
 
+extern void RISCVMarchHelp();
+

craig.topper wrote:
> Can we declare this in RISCVISAInfo.h and include that here?
Yes, we can.



Comment at: clang/tools/driver/cc1_main.cpp:187-188
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();
+

kito-cheng wrote:
> Plz make sure only RISC-V print that, x86 or other target print RISC-V's ext 
> is really weird. 
The check is in clang/lib/Driver/Driver.cpp:4225~



Comment at: clang/tools/driver/cc1_main.cpp:189
+  std::string Error;
+  const llvm::Target *TheTarget =
+  llvm::TargetRegistry::lookupTarget(TargetStr, Error);

craig.topper wrote:
> Why do we need to lookup the TargetRegistry?
I forgot to remove lol~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:187-188
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();
+

Plz make sure only RISC-V print that, x86 or other target print RISC-V's ext is 
really weird. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 516708.
4vtomat added a comment.

Rename clang/test/Driver/print-supported-marchs.c to 
clang/test/Driver/print-supported-extensions.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,6 +139,29 @@
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
+void llvm::riscvMarchHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto  : SupportedExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto  : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef ) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -23,6 +23,8 @@
   unsigned MinorVersion;
 };
 
+void riscvMarchHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,14 @@
   return 0;
 }
 
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();
+
+  return 0;
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -223,6 +232,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return PrintSupportedExtensions(Clang->getTargetOpts().Triple);
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,98 @@
+// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// Test -march=help alias.
+// RUN: %clang --target=riscv64 -march=help 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// CHECK-NOT: warning: argument unused during compilation
+// CHECK-RISCV: Target: riscv64
+// CHECK-NEXT: All available -march extensions for RISC-V
+// CHECK-NEXT: 	NameVersion
+// CHECK-NEXT: 	i   2.0
+// CHECK-NEXT: 	e   1.9
+// CHECK-NEXT: 	m   2.0
+// CHECK-NEXT: 	a   2.0
+// CHECK-NEXT: 	f   2.0
+// CHECK-NEXT: 	d   2.0
+// CHECK-NEXT: 	c   2.0
+// CHECK-NEXT: 	v   1.0
+// CHECK-NEXT: 	h   1.0
+// CHECK-NEXT: 	svinval 1.0
+// CHECK-NEXT: 	svnapot 1.0
+// CHECK-NEXT: 	svpbmt  1.0
+// CHECK-NEXT: 	zicbom  1.0
+// CHECK-NEXT: 	zicbop  1.0
+// CHECK-NEXT: 	zicboz  1.0
+// CHECK-NEXT: 	zicsr   2.0
+// CHECK-NEXT: 	zifencei