[PATCH] D71124: [RISCV] support clang driver to select cpu

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D71124#2423633 , @oceanfish81 wrote:

> @khchen ,
> how should I obtain available (v)CPU features?
> Any sample code, that should work on RISC-V?

Probably `getauxval(AT_HWCAP)`; both Linux and FreeBSD put the ISA string in 
the low 26 bits in the same vein as `misa`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Ivan Serdyuk via Phabricator via cfe-commits]

oceanfish81 added a comment.

@khchen ,
how should I obtain available (v)CPU features?
Any sample code, that should work on RISC-V?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

> How does the CLI options correspond with the back-end capabilities?

`clang -print-supported-cpus`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Ivan Serdyuk via Phabricator via cfe-commits]

oceanfish81 added a comment.

@khchen , what is the recomended way to get the current CPU features?

I see that there are some ISA extensions defined here 
 .

So RISC-V back-end has another API, for both identifying available CPU (or 
vCPU, if under Qemu) features?
How does the CLI options correspond with the back-end capabilities?

There is some code, for gollvm project 
https://go.googlesource.com/gollvm/+/e3ede8ef8009429eaae91bbae2d67542ddaa5853/tools/capture-fcn-attributes.go
 - and it relies on 
https://github.com/llvm/llvm-project/blob/48ddf5e182c61cb93d66325f5690312d9e9226eb/llvm/lib/Support/Host.cpp
 (auto-generation).
I need to understand whethershould Host.cpp be patched or should I use an arch. 
specific/dedicated workaround.

I am forcasting sgnificant contrast in gollvm's adoption, between various 
RISC-V boards, since of an active usage of different CPU features (available on 
one micro-controller, with others unavailable - and vice versa).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

In D71124#2415424 , @oceanfish81 wrote:

> I tried to build the following:
>
> https://gist.github.com/advancedwebdeveloper/09033869445a09a2e297a73387d46c41/raw/bf7dac33e256621c445f9e61f88a832e50d901a3/llvm_cpu_features_investigation.cpp
>
> but it didn't show me any CPU features, on 
> https://fedoraproject.org/wiki/Architectures/RISC-V/Installing#Boot_under_QEMU

It seems 

 `getHostCPUFeatures` does not support RISCV target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Ivan Serdyuk via Phabricator via cfe-commits]

oceanfish81 added a comment.
Herald added subscribers: frasercrmck, dexonsmith, NickHung, pengfei.

I tried to build the following:

https://gist.github.com/advancedwebdeveloper/09033869445a09a2e297a73387d46c41/raw/bf7dac33e256621c445f9e61f88a832e50d901a3/llvm_cpu_features_investigation.cpp

but it didn't show me any CPU features, on 
https://fedoraproject.org/wiki/Architectures/RISC-V/Installing#Boot_under_QEMU


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG294d1eae75bf: [RISCV] Add support for -mcpu option. 
(authored by Zakk Chen zakk.c...@sifive.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-arch.c
  clang/test/Driver/riscv-cpus.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -215,6 +215,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -11,11 +11,12 @@
 //
 //===--===//
 
-#include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/ARMBuildAttributes.h"
 
 using namespace llvm;
 using namespace AMDGPU;
@@ -208,3 +209,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (C.Kind != CK_INVALID && IsRV64 == C.is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned CPUFeatures = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (CPUFeatures == FK_INVALID)
+return false;
+
+  if (CPUFeatures & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -130,6 +130,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
+
+} // namespace RISCV
+
 } // namespace llvm
 
 #endif
Index: 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.

I am happy with this. I think we should get it landed so we can backport it to 
the LLVM 11 branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 278197.
khchen added a comment.

address @lenary's comment, good catch!! thanks!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-arch.c
  clang/test/Driver/riscv-cpus.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -215,6 +215,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -11,11 +11,12 @@
 //
 //===--===//
 
-#include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/ARMBuildAttributes.h"
 
 using namespace llvm;
 using namespace AMDGPU;
@@ -208,3 +209,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (C.Kind != CK_INVALID && IsRV64 == C.is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned CPUFeatures = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (CPUFeatures == FK_INVALID)
+return false;
+
+  if (CPUFeatures & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -130,6 +130,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
+
+} // namespace RISCV
+
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/RISCVTargetParser.def

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Thanks for the fix!

Please can you also update `clang/test/Misc/target-invalid-cpu-note.c` for 
riscv32 and riscv64 - it's likely you'll need the fix in the inline note below.

Also, there are a bunch of clang-tidy issues, which I think we can fix now 
rather than later.




Comment at: llvm/lib/Support/TargetParser.cpp:250
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);

This line should be `if (C.Kind != CK_INVALID && IsRV64 == C.Is64Bit())` so 
that when clang prints out valid CPUs, 'invalid' is not included in the list. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 278106.
khchen marked 8 inline comments as done.
khchen added a comment.

address @asb's comment. thanks.

Thanks for @lenary bug report.
I cannot reproduce the crash in local,
but I though the problem is cause by removing "Choose a default based on the 
triple" logic in `getRISCVABI`
The `MArch` value is "unknown" and there is no return value which can match any 
conditionin.
so I add it back and also update the "invalid arch name" test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-arch.c
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -215,6 +215,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
@@ -208,3 +209,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -130,6 +130,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind kind, std::vector );
+
+} // namespace RISCV
+
 } // 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

I've added some suggestions to clarify the code comments. I think before 
landing it would be good to address the crash Sam pointed out for an invalid 
-march, but otherwise I think this looks good to me (at least, it seems worth 
landing this and if further issues crop up we can fix them and request them for 
merging into the LLVM 11 branch).




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:556
   //
-  // The logic uses the following, in order:
+  // The logic uses in GCC 9.2.0 as the following, in order:
   // 1. Explicit choices using `--with-abi=`

"The logic used in GCC 9.2.0 i the following, in order:"



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:566
+  //
+  // In order to make chosing logic more clear, clang uses the following logic,
+  // in order:

"Clang uses the following logic;"



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:569
+  // 1. Explicit choices using `-mabi=`
+  // 2. A default based on clang's choosing arch logic. The order of default
+  // arch login is `-march`, `-mcpu` and target triples`.

"A default based on the architecture as determined by getRISCVArch"



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:576
 
-  // 2. Choose a default based on `-march=`
+  // 2. Choose a default based on clang's choosing arch logic.
   //

"Choose a default based on the target architecture"



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:615
   //
-  // The logic uses the following, in order:
+  // The logic uses in GCC 9.2.0 as the following, in order:
   // 1. Explicit choices using `--with-arch=`

"The logic used in GCC 9.2.0 is the following, in order:"



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:627
+  // 1. Explicit choices using `-march=`
+  // 2. Based on `-mcpu` if target cpu has default isa extension feature
+  // 3. A default based on `-mabi`, if provided

Based on `-mcpu` if the target CPU has a default ISA string



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.h:29
const llvm::Triple );
+StringRef getMArchFromMabi(StringRef MABI);
+StringRef getMArchFromTriple(const llvm::Triple );

These functions aren't defined anywhere - delete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

Hi @lenary 
This is normal behavior in current clang implementation, 
`-mcpu=?` flag does not interact with any flags, it just print out the all of 
ProcessorModel registered in backend.
https://github.com/llvm/llvm-project/blob/master/clang/tools/driver/cc1_main.cpp#L213-L215

You can reference this 
,
 so the `clang -mcpu=?` result is exactly equal to `./bin/llc -mattr=+cpuhelp`.

  ./bin/llc -mattr=+cpuhelp  foo.ll  --mtriple=riscv32
  Available CPUs for this target:
  
  generic-rv32
  generic-rv64
  rocket-rv32
  rocket-rv64
  
  Use -mcpu or -mtune to specify the target's processor.
  For example, clang --target=aarch64-unknown-linux-gui -mcpu=cortex-a35

You can try `clang -mcpu=? --target=mips64el` or other target too.

But you are right, this behavior is very confusing users.

In D71124#2150535 , @lenary wrote:

> I've got one major issue (inline below), and I'm confused by some other 
> behaviour:
>
> When I run `clang --target=riscv64 -mcpu=?`, the list includes both 
> `generic-rv32` and `generic-rv64`. It doesn't show only the 64-bit cpus. This 
> is not changed by giving a full triple, or by additionally specifying 
> `-march=rv64gc`. Should it be?
>
> For instance, this output:
>
>   $ clang --target=riscv64-unknown-elf -mcpu=\? -march=rv64gc
>   clang version 11.0.0
>   Target: riscv64-unknown-unknown-elf
>   Thread model: posix
>   InstalledDir: /home/selliott/llvm-project/./build/llvm_clang/all/bin
>   Available CPUs for this target:
>  
>   generic-rv32
>   generic-rv64
>   rocket-rv32
>   rocket-rv64
>   sifive-e31
>   sifive-u54
>  
>   Use -mcpu or -mtune to specify the target's processor.
>   For example, clang --target=aarch64-unknown-linux-gui -mcpu=cortex-a35
>





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I've got one major issue (inline below), and I'm confused by some other 
behaviour:

When I run `clang --target=riscv64 -mcpu=?`, the list includes both 
`generic-rv32` and `generic-rv64`. It doesn't show only the 64-bit cpus. This 
is not changed by giving a full triple, or by additionally specifying 
`-march=rv64gc`. Should it be?

For instance, this output:

  $ clang --target=riscv64-unknown-elf -mcpu=\? -march=rv64gc
  clang version 11.0.0
  Target: riscv64-unknown-unknown-elf
  Thread model: posix
  InstalledDir: /home/selliott/llvm-project/./build/llvm_clang/all/bin
  Available CPUs for this target:
  
generic-rv32
generic-rv64
rocket-rv32
rocket-rv64
sifive-e31
sifive-u54
  
  Use -mcpu or -mtune to specify the target's processor.
  For example, clang --target=aarch64-unknown-linux-gui -mcpu=cortex-a35




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:599
+  return "lp64";
   }
 }

I now get a stacktrace here if I call `clang --target=riscv32 -march=unknown 
foo.c -c`. In clang-10, you get a nice error:
```
clang-10: error: invalid arch name 'wat', string must begin with rv32{i,e,g} or 
rv64{i,g}
```

The stacktrace is coming from the call to `getRISCVABI` in 
`findRISCVBareMetalMultilibs` (in `clang/lib/Driver/ToolChains/Gnu.cpp:1608`).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

@asb @lenary I thought this path is ready to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 277067.
khchen added a comment.

avoid to check compiler version in testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -215,6 +215,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
@@ -208,3 +209,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -130,6 +130,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind kind, std::vector );
+
+} // namespace RISCV
+
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/RISCVTargetParser.def
===
--- /dev/null
+++ llvm/include/llvm/Support/RISCVTargetParser.def
@@ -0,0 +1,13 @@
+#ifndef PROC
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)
+#endif
+
+PROC(INVALID, {"invalid"}, FK_INVALID, {""})

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 277042.
khchen added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -215,6 +215,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
@@ -208,3 +209,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -130,6 +130,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind kind, std::vector );
+
+} // namespace RISCV
+
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/RISCVTargetParser.def
===
--- /dev/null
+++ llvm/include/llvm/Support/RISCVTargetParser.def
@@ -0,0 +1,13 @@
+#ifndef PROC
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)
+#endif
+
+PROC(INVALID, {"invalid"}, FK_INVALID, {""})
+PROC(GENERIC_RV32, {"generic-rv32"}, 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 276951.
khchen added a comment.

addess asb's comment.

[RISCV][Clang] Support -mcpu option.

Summary:

1. gcc uses `-march` and `-mtune` flag to chose arch and

pipeline model, but clang does not have `-mtune` flag,
we uses `-mcpu` to chose both info.

2. Add SiFive e31 and u54 cpu which have default march

and pipeline model.

3. Specific `-mcpu` with rocket-rv[32|64] would select

pipeline model only, and use the driver's arch choosing
logic to get default arch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -215,6 +215,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
@@ -208,3 +209,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -130,6 +130,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind kind, std::vector );

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen planned changes to this revision.
khchen marked an inline comment as done.
khchen added a comment.

BTW, this patch depends on D77030 , which aim 
to avoid the forcing of any ProcessorModel to have `FeatureRVCHints` feature.
But if we decide to keep the `FeatureRVCHints`, I need to change implementation 
a little.




Comment at: clang/test/Driver/riscv-cpus.c:2
+// Check target CPUs are correctly passed.
+
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck 
-check-prefix=MCPU-ROCKETCHIP32 %s

asb wrote:
> I think for completeness this test should be validating the interaction of 
> the ABI choosing logic with CPU selection as well. With the implemented logic 
> I believe it should show that lp64d is selected for -mcpu=sifive-u54 and that 
> -mcpu=sifive-u54 -mabi=lp64 will respect the ABI choice
okay, it makes sense to me, I will update this patch soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

This has been hanging around for a while, but I think we'd basically agreed 
this is the right logic. The comments have ended up referring to flags that 
don't exist on Clang making it a little hard to follow, and I've added a 
request to slightly expand testing. If you make those cleanups I think it 
should be ready for a final review and merge.

As Sam says, lets flag this in today's RISC-V LLVM call to double-check 
everyone is happy.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:622
+  // 1. Explicit choices using `--with-arch=`
+  // 2. Based on `-mcpu` if target cpu has default isa extension feature
+  // 3. A default based on `--with-abi=`, if provided

As clang has no with-arch or with-abi, this comment seems inaccurate?



Comment at: clang/test/Driver/riscv-cpus.c:2
+// Check target CPUs are correctly passed.
+
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck 
-check-prefix=MCPU-ROCKETCHIP32 %s

I think for completeness this test should be validating the interaction of the 
ABI choosing logic with CPU selection as well. With the implemented logic I 
believe it should show that lp64d is selected for -mcpu=sifive-u54 and that 
-mcpu=sifive-u54 -mabi=lp64 will respect the ABI choice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I realise this is almost certainly something we want to land before the LLVM 11 
branch date, as we included schedules in LLVM 10 with no way to use them, and 
would like users to be able to use them. I'll bring it up on the call tomorrow 
- I hope this PR implements what we agreed from the previous calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 261741.
khchen added a comment.

address kito's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -207,6 +207,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
@@ -206,3 +207,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -172,6 +172,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind kind, std::vector );
+
+} // namespace RISCV
+
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/RISCVTargetParser.def
===
--- /dev/null
+++ llvm/include/llvm/Support/RISCVTargetParser.def
@@ -0,0 +1,13 @@
+#ifndef PROC
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)
+#endif
+
+PROC(INVALID, {"invalid"}, FK_INVALID, {""})
+PROC(GENERIC_RV32, 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:478
   // 1. Explicit choices using `--with-abi=`
-  // 2. A default based on `--with-arch=`, if provided
-  // 3. A default based on the target triple's arch
+  // 2. A default based on arch
   //

I think it worth to keep this comment untouched, since it describing GCC's 
behavior.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:531
+  // 3. A default based on `--with-abi=`, if provided
+  // 4. A default based on the target triple's arch
   //

This part seems like is describing about GCC's behavior, so it should not 
change unless GCC changed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

In D71124#2012090 , @kito-cheng wrote:

> Another proposal for -mcpu and -mtune:
>
> Decoupling the -mcpu and -mtune option, -mcpu only accept concrete CPU, and 
> -mtune for micro-arch/pipeline model, they accept different option set.
>
> e.g.
>  -mcpu=sifive-e24 # Imply -march=rv32imafc -mtune=sifive-2-series
>  -mtune=sifive-2-series # no effect on arch
>  -mtune=rocket # no effect on arch
>
> So -mcpu=rocket is invalid, since it's micro-arch/pipeline model only,
>  and -mtune=sifive-e24 is invalid too, because it's CPU not a micro-arch.


I agree this is better proposal but clang treats -mtune and -mcpu as same 
options now...
ex. 
https://github.com/llvm-mirror/clang/blob/master/include/clang/Driver/Options.td#L2751-L2752


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

In D71124#2010759 , @lenary wrote:

> This is looking good.
>
> I remember we discussed this on the LLVM call a few weeks ago - there was a 
> discussion as to whether we should be prioritising `-march` or `-mcpu` - do 
> you recall the outcome of that discussion?


Sorry, I don't remember the outcome of discussion...
I thought it make sense if both mcpu and march are specified, and march 
extensions overrides the mcpu extension
This also allows users to specific superset or subset of march.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:501-516
-  // 3. Choose a default based on the triple
-  //
-  // We deviate from GCC's defaults here:
-  // - On `riscv{XLEN}-unknown-elf` we use the integer calling convention only.
-  // - On all other OSs we use the double floating point calling convention.
-  if (Triple.getArch() == llvm::Triple::riscv32) {
-if (Triple.getOS() == llvm::Triple::UnknownOS)

lenary wrote:
> What's the justification for removing this code?
I changed the logic in how to get the default march by calling 
`getRISCVArch(Args, Triple);`
`getRISCVArch` can also get the default march based on the triple, so we don't 
need above code now.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:310
+  case llvm::Triple::riscv64:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();

lenary wrote:
> Should we be doing more validation here?
I think we don't need to validation here, we have validation in 
RISCVTargetInfo::isValidCPUName. You can see it work in 
test/Driver/riscv-cpus.c#L22.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 261165.
khchen marked 9 inline comments as done.
khchen added a comment.

address @lenary's review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -115,6 +115,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
@@ -206,3 +207,64 @@
   default: return {0, 0, 0};
   }
 }
+
+namespace llvm {
+namespace RISCV {
+
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)  \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
+bool checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+CPUKind parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool getCPUFeaturesExceptStdExt(CPUKind Kind,
+std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+  else
+Features.push_back("-64bit");
+
+  return true;
+}
+
+} // namespace RISCV
+} // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -169,6 +169,32 @@
 
 } // namespace AMDGPU
 
+namespace RISCV {
+
+enum CPUKind : unsigned {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
+#include "RISCVTargetParser.def"
+};
+
+enum FeatureKind : unsigned {
+  FK_INVALID = 0,
+  FK_NONE = 1,
+  FK_STDEXTM = 1 << 2,
+  FK_STDEXTA = 1 << 3,
+  FK_STDEXTF = 1 << 4,
+  FK_STDEXTD = 1 << 5,
+  FK_STDEXTC = 1 << 6,
+  FK_64BIT = 1 << 7,
+};
+
+bool checkCPUKind(CPUKind kind, bool IsRV64);
+CPUKind parseCPUKind(StringRef CPU);
+StringRef getMArchFromMcpu(StringRef CPU);
+void fillValidCPUArchList(SmallVectorImpl , bool IsRV64);
+bool getCPUFeaturesExceptStdExt(CPUKind kind, std::vector );
+
+} // namespace RISCV
+
 } // namespace llvm
 
 #endif
Index: llvm/include/llvm/Support/RISCVTargetParser.def
===
--- /dev/null
+++ llvm/include/llvm/Support/RISCVTargetParser.def
@@ -0,0 +1,13 @@
+#ifndef PROC
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH)
+#endif
+
+PROC(INVALID, 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Another proposal for -mcpu and -mtune:

Decoupling the -mcpu and -mtune option, -mcpu only accept concrete CPU, and 
-mtune for micro-arch/pipeline model, they accept different option set.

e.g.
-mcpu=sifive-e24 # Imply -march=rv32imafc -mtune=sifive-2-series
-mtune=sifive-2-series # no effect on arch
-mtune=rocket # no effect on arch

So -mcpu=rocket is invalid, since it's micro-arch/pipeline model only,
and -mtune=sifive-e24 is invalid too, because it's CPU not a micro-arch.


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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is looking good.

I remember we discussed this on the LLVM call a few weeks ago - there was a 
discussion as to whether we should be prioritising `-march` or `-mcpu` - do you 
recall the outcome of that discussion?




Comment at: clang/lib/Basic/Targets/RISCV.cpp:166
+bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
+  return llvm::RISCV::checkCPUKind(llvm::RISCV::parseCPUKind(Name), false);
+}

It would be good to have `/*Is64Bit=*/` before the boolean arguments to these 
functions.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:501-516
-  // 3. Choose a default based on the triple
-  //
-  // We deviate from GCC's defaults here:
-  // - On `riscv{XLEN}-unknown-elf` we use the integer calling convention only.
-  // - On all other OSs we use the double floating point calling convention.
-  if (Triple.getArch() == llvm::Triple::riscv32) {
-if (Triple.getOS() == llvm::Triple::UnknownOS)

What's the justification for removing this code?



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:310
+  case llvm::Triple::riscv64:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();

Should we be doing more validation here?



Comment at: llvm/lib/Support/TargetParser.cpp:122
 
+struct CPUInfo {
+  StringLiteral Name;

I think this should be prefixed `RISCV`, or moved into the RISCV namespace if 
you can?



Comment at: llvm/lib/Support/TargetParser.cpp:259-260
+
+  if (features & FK_64BIT)
+Features.push_back("+64bit");
+

It might be worth explicitly adding `-64bit` where FK_64BIT is not set.


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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

@lenary (Sorry for the very late reply...)

In D71124#1829998 , @lenary wrote:

> In D71124#1792216 , @khchen wrote:
>
> > The problem is how `-mcpu` interact with explicitly specified `-march` (or 
> > target features).
> >
> > 1. in GCC, -mcpu is only used to chose the pipeline model,
>
>
> I think you mean "in GCC, `-mtune` is only used to choose the pipeline model" 
> (`-mcpu` is not documented in the RISC-V specific GCC options documentation 
> ).


Yes, It's my mistake, I mean `-mtune`.

> Clang should attempt to maintain compatibility with GCC flags, but if they 
> only implement `-mtune`, then we have a little more freedom to do something 
> ergonomic with `-mcpu`.
> 
> I'll note that clang already has a large TODO around implementing `-mtune` in 
> general, though the AArch64 backend seems to support it for some option 
> choices.
> 
>> 
>> 
>> 2. I also read this 
>> 
>>  article talking about the X86 and ARM to handle those options.
>>   - -march=X: Tells the compiler that X is the minimal architecture the 
>> binary must run on.  The compiler is free to use architecture-specific 
>> instructions.  This flag behaves differently on Arm and x86.  On Arm, -march 
>> does not override -mtune, but on x86 -march will override both -mtune and 
>> -mcpu.
>>   - -mtune=X: Tells the compiler to optimize for microarchitecture X but 
>> does not allow the compiler to change the ABI or make assumptions about 
>> available instructions.  This flag has the more-or-less the same meaning on 
>> Arm and x86.
>>   - -mcpu=X: On Arm, this flag is a combination of -march and -mtune.  It 
>> simultaneously specifies the target architecture and optimizes for a given 
>> microarchitecture. On x86 this flag is a deprecated synonym for -mtune.
>> 
>> So maybe it makes sense to treat those flags behaves differently on 
>> different target .
>> 3. I also tried llc to specific -mcpu and -attr (similar to -march, 
>> target-feature) in ARM, -attr will over write the default target-feature in 
>> -mcpu.
>> 
>>   on RISC-V, in sometime (or most?) we have same pipeline model but support 
>> different extension combination,
> 
> I don't believe this to be correct. lowRISC's Ibex has a completely different 
> pipeline model to rocket, and there are countless other RISC-V cores with 
> different pipeline characteristics, including out-of-order pipeline 
> implementations like BOOM. I don't think we can favour one particular 
> scheduling model (beyond the generic ones we already default to).

Yes, so lowRISC's Ibex need to use different pipleine model.  
I mention it just because in SiFive, the same serial core can share the same 
schedule model, but they have different isa extension.

>> so I think maybe distinguishing the purpose of -mcpu and -march and make 
>> them with no interaction is a good idea. (behavior is equal to GCC)
> 
> In LLVM, if you add `target-cpu` metadata to a function (which is added by 
> clang, based on `-mcpu`), that function will have all the features of that 
> CPU automatically added to it (as if you had used `-mattr` with all the 
> features in the model). If you don't add that metadata, a generic scheduling 
> model will be chosen. This suggests at the moment there can be no separation 
> between `-mtune` and `-march` as there is in GCC (without changes to the 
> target-independent parts of LLVM).

thanks for reminding me, I re-implementation it.

In D71124#1829998 , @lenary wrote:

> In D71124#1792216 , @khchen wrote:
>
> > The problem is how `-mcpu` interact with explicitly specified `-march` (or 
> > target features).
> >
> > 1. in GCC, -mcpu is only used to chose the pipeline model,
>
>
> I think you mean "in GCC, `-mtune` is only used to choose the pipeline model" 
> (`-mcpu` is not documented in the RISC-V specific GCC options documentation 
> ).
>
> Clang should attempt to maintain compatibility with GCC flags, but if they 
> only implement `-mtune`, then we have a little more freedom to do something 
> ergonomic with `-mcpu`.
>
> I'll note that clang already has a large TODO around implementing `-mtune` in 
> general, though the AArch64 backend seems to support it for some option 
> choices.
>
> > 
> > 
> > 2. I also read this 
> > 
> >  article talking about the X86 and ARM to handle those options.
> >   - -march=X: Tells the compiler that X is the minimal architecture the 
> > binary must run on.  

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 255377.
khchen added reviewers: evandro, HsiangKai.
khchen added a comment.
Herald added a subscriber: hiraditya.

update implementation and handle `-mcpu` with explicitly specified `-march`.
If `-mcpu` has default march, explicitly `-march` will overwrite it.

1. reference ARM/AArch64 and X86 to implement RISCVTargetParser.def
2. in clang, `-mtune` option alias 

 to `-mcpu`. This patch supports `-mtune` option via `-mcpu`.
3. propose two category of CPUs in TargetParser and backend. First is similar 
gcc's -mtune option (it includes micro architecture related feature only, ex. 
`rocket-rv32` has different schedule model), another also has default `-march` 
(isa string).
4. add two sifive cpu which has default `-march`
5. changed default march logic.

march: in order:

1. Explicit choices using -march
2. A default based on `-mcpu`, if target cpu has default -march
3. A default based on -mabi, if provided
4. A default based on the target triple's arch

I think it's also okay if we want to only support `-mcpu` which is similar to 
gcc's `-mtune` in current stage.  Because in current implementation,
users can query available CPUs via ` -mcpu=?` on clang but `mcpu` option is not 
workable.


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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/RISCV.td

Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -115,6 +115,16 @@
 
 def : ProcessorModel<"rocket-rv64", Rocket64Model, [Feature64Bit]>;
 
+def : ProcessorModel<"sifive-e31", Rocket32Model, [FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtC]>;
+
+def : ProcessorModel<"sifive-u54", Rocket64Model, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,
+   FeatureStdExtF,
+   FeatureStdExtD,
+   FeatureStdExtC]>;
 
 //===--===//
 // Define the RISC-V target.
Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -13,12 +13,14 @@
 
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 
 using namespace llvm;
 using namespace AMDGPU;
+using namespace RISCV;
 
 namespace {
 
@@ -117,6 +119,20 @@
   return I;
 }
 
+struct CPUInfo {
+  StringLiteral Name;
+  CPUKind Kind;
+  unsigned Features;
+  StringLiteral DefaultMarch;
+  bool Is64Bit() const { return (Features & FK_64BIT); }
+};
+
+constexpr CPUInfo RISCVCPUInfo[] = {
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) \
+  {NAME, CK_##ENUM, FEATURES, DEFAULT_MARCH},
+#include "llvm/Support/RISCVTargetParser.def"
+};
+
 } // namespace
 
 StringRef llvm::AMDGPU::getArchNameAMDGCN(GPUKind AK) {
@@ -206,3 +222,42 @@
   default: return {0, 0, 0};
   }
 }
+
+bool RISCV::checkCPUKind(CPUKind Kind, bool IsRV64) {
+  if (Kind == CK_INVALID)
+return false;
+  return RISCVCPUInfo[static_cast(Kind)].Is64Bit() == IsRV64;
+}
+
+RISCV::CPUKind RISCV::parseCPUKind(StringRef CPU) {
+  return llvm::StringSwitch(CPU)
+#define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) .Case(NAME, CK_##ENUM)
+#include "llvm/Support/RISCVTargetParser.def"
+  .Default(CK_INVALID);
+}
+
+StringRef RISCV::getMArchFromMcpu(StringRef CPU) {
+  CPUKind Kind = parseCPUKind(CPU);
+  return RISCVCPUInfo[static_cast(Kind)].DefaultMarch;
+}
+
+void RISCV::fillValidCPUArchList(SmallVectorImpl , bool IsRV64) {
+  for (const auto  : RISCVCPUInfo) {
+if (IsRV64 == C.Is64Bit())
+  Values.emplace_back(C.Name);
+  }
+}
+
+// Get all features except standard extension feature
+bool RISCV::getCPUFeaturesExceptStdExt(CPUKind Kind,
+   std::vector ) {
+  unsigned features = RISCVCPUInfo[static_cast(Kind)].Features;
+
+  if (features == FK_INVALID)
+return false;
+
+ 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D71124#1792216 , @khchen wrote:

> The problem is how `-mcpu` interact with explicitly specified `-march` (or 
> target features).
>
> 1. in GCC, -mcpu is only used to chose the pipeline model,


I think you mean "in GCC, `-mtune` is only used to choose the pipeline model" 
(`-mcpu` is not documented in the RISC-V specific GCC options documentation 
).

Clang should attempt to maintain compatibility with GCC flags, but if they only 
implement `-mtune`, then we have a little more freedom to do something 
ergonomic with `-mcpu`.

I'll note that clang already has a large TODO around implementing `-mtune` in 
general, though the AArch64 backend seems to support it for some option choices.

> 
> 
> 2. I also read this 
> 
>  article talking about the X86 and ARM to handle those options.
>   - -march=X: Tells the compiler that X is the minimal architecture the 
> binary must run on.  The compiler is free to use architecture-specific 
> instructions.  This flag behaves differently on Arm and x86.  On Arm, -march 
> does not override -mtune, but on x86 -march will override both -mtune and 
> -mcpu.
>   - -mtune=X: Tells the compiler to optimize for microarchitecture X but does 
> not allow the compiler to change the ABI or make assumptions about available 
> instructions.  This flag has the more-or-less the same meaning on Arm and x86.
>   - -mcpu=X: On Arm, this flag is a combination of -march and -mtune.  It 
> simultaneously specifies the target architecture and optimizes for a given 
> microarchitecture. On x86 this flag is a deprecated synonym for -mtune.
> 
> So maybe it makes sense to treat those flags behaves differently on 
> different target .
> 3. I also tried llc to specific -mcpu and -attr (similar to -march, 
> target-feature) in ARM, -attr will over write the default target-feature in 
> -mcpu.
> 
>   on RISC-V, in sometime (or most?) we have same pipeline model but support 
> different extension combination,

I don't believe this to be correct. lowRISC's Ibex has a completely different 
pipeline model to rocket, and there are countless other RISC-V cores with 
different pipeline characteristics, including out-of-order pipeline 
implementations like BOOM. I don't think we can favour one particular 
scheduling model (beyond the generic ones we already default to).

> so I think maybe distinguishing the purpose of -mcpu and -march and make them 
> with no interaction is a good idea. (behavior is equal to GCC)

In LLVM, if you add `target-cpu` metadata to a function (which is added by 
clang, based on `-mcpu`), that function will have all the features of that CPU 
automatically added to it (as if you had used `-mattr` with all the features in 
the model). If you don't add that metadata, a generic scheduling model will be 
chosen. This suggests at the moment there can be no separation between `-mtune` 
and `-march` as there is in GCC (without changes to the target-independent 
parts of LLVM).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Evandro Menezes via Phabricator via cfe-commits]

evandro added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};

Strange formatting...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen marked an inline comment as done.
khchen added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};

lenary wrote:
> khchen wrote:
> > lenary wrote:
> > > Is there not a tablegen'd implementation of these based on 
> > > https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/RISCV/RISCV.td#L96-L99
> > >  (which will include `rocket-rv32` and `rocket-rv64` when those two 
> > > schedules are landed)?
> > you are right, if generic-cpu uses rocket chip scheduler, it's okay to 
> > abandon this patch.
> > 
> >  
> No, that's not quite what I mean. When we add the rocket schedules, there 
> will be additional `ProcessorModel` entries for the rocket chips, in addition 
> to the current generic models. 
> 
> The point in these `ProcessorModel` entries is if a user passes 
> `-mcpu=generic-rv64`, then `[Feature64Bit, FeatureRVCHints]` will get turned 
> on, because they chose a specific cpu. This is different to validating that 
> the correct features are enabled in order to choose a cpu, which seems the 
> correct way around.
> 
> Then instead of checking against hard-coded lists, you use use 
> `MCSubtargetInfo::isCPUStringValid(StringRef)`, which uses the info from the 
> `ProcessorModel` tablegen entries. 
@lenary 
I see no backend uses the info (ex. RISCVSubTypeKV?) from tablegen entries, 
I saw different targets use different way to impl.  
`TargetInfo::isValidCPUName`.
for example:
x86 uses [[ 
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/X86Target.def#L9
 | clang/Basic/X86Target.def]] to record (hard-codeed) valid cpu enum and 
string, 
arm/aarch64 uses [[ 
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/ARMTargetParser.def#L184
 | llvm/Support/ARMTargetParser.def]],
and mips just hard code the valid cpu string in [[ 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Basic/Targets/Mips.cpp#L47
 | clang/lib/Basic/Targets/Mips.cpp]].
They don't use backend to check valid cpu string, so maybe this patch is 
workable.
But I wonder if you are asking this patch should implement the checking for 
invalid combination like `-mcpu=generic-rv32` with `-mattr=+64bit` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen planned changes to this revision.
khchen marked an inline comment as done.
khchen added a comment.

The problem is how `-mcpu` interact with explicitly specified `-march` (or 
target features).

1. in GCC, -mcpu is only used to chose the pipeline model,



2. I also read this 

 article talking about the X86 and ARM to handle those options.

-march=X: Tells the compiler that X is the minimal architecture the binary must 
run on.  The compiler is free to use architecture-specific instructions.  This 
flag behaves differently on Arm and x86.  On Arm, -march does not override 
-mtune, but on x86 -march will override both -mtune and -mcpu.
-mtune=X: Tells the compiler to optimize for microarchitecture X but does not 
allow the compiler to change the ABI or make assumptions about available 
instructions.  This flag has the more-or-less the same meaning on Arm and x86. 
-mcpu=X: On Arm, this flag is a combination of -march and -mtune.  It 
simultaneously specifies the target architecture and optimizes for a given 
microarchitecture. On x86 this flag is a deprecated synonym for -mtune.

So maybe it makes sense to treat those flags behaves differently on different 
target .

3. I also tried llc to specific -mcpu and -attr (similar to -march, 
target-feature) in ARM, -attr will over write the default target-feature in 
-mcpu.

on RISC-V, in sometime (or most?) we have same pipeline model but support 
different extension combination, 
so I think maybe distinguishing the purpose of -mcpu and -march and make them 
with no interaction is a good idea. (behavior is equal to GCC)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};

khchen wrote:
> lenary wrote:
> > Is there not a tablegen'd implementation of these based on 
> > https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/RISCV/RISCV.td#L96-L99
> >  (which will include `rocket-rv32` and `rocket-rv64` when those two 
> > schedules are landed)?
> you are right, if generic-cpu uses rocket chip scheduler, it's okay to 
> abandon this patch.
> 
>  
No, that's not quite what I mean. When we add the rocket schedules, there will 
be additional `ProcessorModel` entries for the rocket chips, in addition to the 
current generic models. 

The point in these `ProcessorModel` entries is if a user passes 
`-mcpu=generic-rv64`, then `[Feature64Bit, FeatureRVCHints]` will get turned 
on, because they chose a specific cpu. This is different to validating that the 
correct features are enabled in order to choose a cpu, which seems the correct 
way around.

Then instead of checking against hard-coded lists, you use use 
`MCSubtargetInfo::isCPUStringValid(StringRef)`, which uses the info from the 
`ProcessorModel` tablegen entries. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen marked 3 inline comments as done.
khchen added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};

lenary wrote:
> Is there not a tablegen'd implementation of these based on 
> https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/RISCV/RISCV.td#L96-L99
>  (which will include `rocket-rv32` and `rocket-rv64` when those two schedules 
> are landed)?
you are right, if generic-cpu uses rocket chip scheduler, it's okay to abandon 
this patch.

 



Comment at: clang/lib/Basic/Targets/RISCV.h:46
 
+  bool setCPU(const std::string ) override {
+CPU = Name;

Jim wrote:
> I think this should test cpu name is valid first. And assign Name to CPU, if 
> it is valid.
yes, you are right, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 233082.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c


Index: clang/test/Driver/riscv-cpus.c
===
--- /dev/null
+++ clang/test/Driver/riscv-cpus.c
@@ -0,0 +1,8 @@
+// Check target CPUs are correctly passed.
+
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck 
-check-prefix=ROCKETCHIP32 %s
+// ROCKETCHIP32: "-fuse-init-array" "-target-cpu" "rocket-rv32"
+
+// RUN: %clang -target riscv64 -### -c %s 2>&1 -mcpu=rocket-rv64 | FileCheck 
-check-prefix=ROCKETCHIP64 %s
+// ROCKETCHIP64: "-fuse-init-array" "-target-cpu" "rocket-rv64"
+
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -304,6 +304,11 @@
 }
 return TargetCPUName;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();
+return "";
 
   case llvm::Triple::bpfel:
   case llvm::Triple::bpfeb:
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -24,7 +24,7 @@
 // RISC-V Target
 class RISCVTargetInfo : public TargetInfo {
 protected:
-  std::string ABI;
+  std::string ABI, CPU;
   bool HasM;
   bool HasA;
   bool HasF;
@@ -43,6 +43,13 @@
 WIntType = UnsignedInt;
   }
 
+  bool setCPU(const std::string ) override {
+if (!isValidCPUName(Name))
+  return false;
+CPU = Name;
+return true;
+  }
+
   StringRef getABI() const override { return ABI; }
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
@@ -94,6 +101,7 @@
 return false;
   }
 
+  bool isValidCPUName(StringRef Name) const override;
   void setMaxAtomicWidth() override {
 MaxAtomicPromoteWidth = 128;
 
@@ -118,6 +126,7 @@
 return false;
   }
 
+  bool isValidCPUName(StringRef Name) const override;
   void setMaxAtomicWidth() override {
 MaxAtomicPromoteWidth = 128;
 
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -160,3 +160,17 @@
 
   return true;
 }
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};
+
+bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
+  return llvm::find(ValidRV32CPUNames, Name) != std::end(ValidRV32CPUNames);
+}
+
+static constexpr llvm::StringLiteral ValidRV64CPUNames[] = {{"generic-rv64"},
+{"rocket-rv64"}};
+
+bool RISCV64TargetInfo::isValidCPUName(StringRef Name) const {
+  return llvm::find(ValidRV64CPUNames, Name) != std::end(ValidRV64CPUNames);
+}


Index: clang/test/Driver/riscv-cpus.c
===
--- /dev/null
+++ clang/test/Driver/riscv-cpus.c
@@ -0,0 +1,8 @@
+// Check target CPUs are correctly passed.
+
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck -check-prefix=ROCKETCHIP32 %s
+// ROCKETCHIP32: "-fuse-init-array" "-target-cpu" "rocket-rv32"
+
+// RUN: %clang -target riscv64 -### -c %s 2>&1 -mcpu=rocket-rv64 | FileCheck -check-prefix=ROCKETCHIP64 %s
+// ROCKETCHIP64: "-fuse-init-array" "-target-cpu" "rocket-rv64"
+
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -304,6 +304,11 @@
 }
 return TargetCPUName;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();
+return "";
 
   case llvm::Triple::bpfel:
   case llvm::Triple::bpfeb:
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -24,7 +24,7 @@
 // RISC-V Target
 class RISCVTargetInfo : public TargetInfo {
 protected:
-  std::string ABI;
+  std::string ABI, CPU;
   bool HasM;
   bool HasA;
   bool HasF;
@@ -43,6 +43,13 @@
 WIntType = UnsignedInt;
   }
 
+  bool setCPU(const std::string ) override {
+if (!isValidCPUName(Name))
+  return false;
+CPU = Name;
+return true;
+  }
+
   StringRef getABI() const override { return ABI; }
   

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Sam Elliott via Phabricator via cfe-commits]

lenary added reviewers: lenary, asb.
lenary added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};

Is there not a tablegen'd implementation of these based on 
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/RISCV/RISCV.td#L96-L99
 (which will include `rocket-rv32` and `rocket-rv64` when those two schedules 
are landed)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Jim Lin via Phabricator via cfe-commits]

Jim added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.h:46
 
+  bool setCPU(const std::string ) override {
+CPU = Name;

I think this should test cpu name is valid first. And assign Name to CPU, if it 
is valid.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71124



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen created this revision.
khchen added a project: clang.
Herald added subscribers: cfe-commits, luismarques, apazos, sameer.abuasal, 
pzheng, s.egerton, lenary, Jim, benna, psnobl, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, MaskRay, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb.

This is follow up patch for https://reviews.llvm.org/D68685


Repository:
  rC Clang

https://reviews.llvm.org/D71124

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c


Index: clang/test/Driver/riscv-cpus.c
===
--- /dev/null
+++ clang/test/Driver/riscv-cpus.c
@@ -0,0 +1,8 @@
+// Check target CPUs are correctly passed.
+
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck 
-check-prefix=ROCKETCHIP32 %s
+// ROCKETCHIP32: "-fuse-init-array" "-target-cpu" "rocket-rv32"
+
+// RUN: %clang -target riscv64 -### -c %s 2>&1 -mcpu=rocket-rv64 | FileCheck 
-check-prefix=ROCKETCHIP64 %s
+// ROCKETCHIP64: "-fuse-init-array" "-target-cpu" "rocket-rv64"
+
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -304,6 +304,11 @@
 }
 return TargetCPUName;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();
+return "";
 
   case llvm::Triple::bpfel:
   case llvm::Triple::bpfeb:
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -24,7 +24,7 @@
 // RISC-V Target
 class RISCVTargetInfo : public TargetInfo {
 protected:
-  std::string ABI;
+  std::string ABI, CPU;
   bool HasM;
   bool HasA;
   bool HasF;
@@ -43,6 +43,11 @@
 WIntType = UnsignedInt;
   }
 
+  bool setCPU(const std::string ) override {
+CPU = Name;
+return isValidCPUName(Name);
+  }
+
   StringRef getABI() const override { return ABI; }
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
@@ -94,6 +99,7 @@
 return false;
   }
 
+  bool isValidCPUName(StringRef Name) const override;
   void setMaxAtomicWidth() override {
 MaxAtomicPromoteWidth = 128;
 
@@ -118,6 +124,7 @@
 return false;
   }
 
+  bool isValidCPUName(StringRef Name) const override;
   void setMaxAtomicWidth() override {
 MaxAtomicPromoteWidth = 128;
 
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -160,3 +160,17 @@
 
   return true;
 }
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};
+
+bool RISCV32TargetInfo::isValidCPUName(StringRef Name) const {
+  return llvm::find(ValidRV32CPUNames, Name) != std::end(ValidRV32CPUNames);
+}
+
+static constexpr llvm::StringLiteral ValidRV64CPUNames[] = {{"generic-rv64"},
+{"rocket-rv64"}};
+
+bool RISCV64TargetInfo::isValidCPUName(StringRef Name) const {
+  return llvm::find(ValidRV64CPUNames, Name) != std::end(ValidRV64CPUNames);
+}


Index: clang/test/Driver/riscv-cpus.c
===
--- /dev/null
+++ clang/test/Driver/riscv-cpus.c
@@ -0,0 +1,8 @@
+// Check target CPUs are correctly passed.
+
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck -check-prefix=ROCKETCHIP32 %s
+// ROCKETCHIP32: "-fuse-init-array" "-target-cpu" "rocket-rv32"
+
+// RUN: %clang -target riscv64 -### -c %s 2>&1 -mcpu=rocket-rv64 | FileCheck -check-prefix=ROCKETCHIP64 %s
+// ROCKETCHIP64: "-fuse-init-array" "-target-cpu" "rocket-rv64"
+
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -304,6 +304,11 @@
 }
 return TargetCPUName;
   }
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();
+return "";
 
   case llvm::Triple::bpfel:
   case llvm::Triple::bpfeb:
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -24,7 +24,7 @@
 // RISC-V Target
 class RISCVTargetInfo : public TargetInfo {
 protected:
-  std::string ABI;
+  std::string ABI, CPU;
   bool HasM;
   bool HasA;