Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Add processor definition for XiangShan-NanHu #70294

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Oct 26, 2023

This PR adds the processor definition for XiangShan-NanHu, an open-source high-performance RISC-V processor.

According to the official documentation, NanHu core supports RV64IMAFDC_zba_zbb_zbc_zbs_zbkb_zbkc_zbkx_zknd_zkne_zknh_zksed_zksh_svinval. I found that NanHu also supports zicbom and zicboz. You can find them in the source code. Features supported by NanHu have been confirmed by @poemonsense.

See also #70232.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 26, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 26, 2023

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-clang-driver

Author: Yingwei Zheng (dtcxzyw)

Changes

This PR adds the processor definition for XiangShan-NanHu, an open-source high-performance RISC-V processor.

According to the official documentation, NanHu core supports RV64IMAFDC_zba_zbb_zbc_zbs_zbkb_zbkc_zbkx_zknd_zkne_zknh_zksed_zksh_svinval. I found that NanHu also supports zicbom and zicboz. You can find them in the source code. Features supported by NanHu have been confirmed by @poemonsense.

See also #70232.


Full diff: https://github.com/llvm/llvm-project/pull/70294.diff

3 Files Affected:

  • (modified) clang/test/Driver/riscv-cpus.c (+14)
  • (modified) clang/test/Misc/target-invalid-cpu-note.c (+2-2)
  • (modified) llvm/lib/Target/RISCV/RISCVProcessors.td (+21)
diff --git a/clang/test/Driver/riscv-cpus.c b/clang/test/Driver/riscv-cpus.c
index 3eaceedce685fc6..70f0a63336bd478 100644
--- a/clang/test/Driver/riscv-cpus.c
+++ b/clang/test/Driver/riscv-cpus.c
@@ -20,6 +20,17 @@
 // MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
 // MCPU-SYNTACORE-SCR1-MAX: "-target-abi" "ilp32"
 
+// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=xiangshan-nanhu | FileCheck -check-prefix=MCPU-XIANGSHAN-NANHU %s
+// MCPU-XIANGSHAN-NANHU: "-nostdsysteminc" "-target-cpu" "xiangshan-nanhu"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+m" "-target-feature" "+a" "-target-feature" "+f" "-target-feature" "+d"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+c"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+zicbom" "-target-feature" "+zicboz" "-target-feature" "+zicsr" "-target-feature" "+zifencei"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+zba" "-target-feature" "+zbb" "-target-feature" "+zbc"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+zbkb" "-target-feature" "+zbkc" "-target-feature" "+zbkx" "-target-feature" "+zbs"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+zkn" "-target-feature" "+zknd" "-target-feature" "+zkne" "-target-feature" "+zknh"
+// MCPU-XIANGSHAN-NANHU: "-target-feature" "+zks" "-target-feature" "+zksed" "-target-feature" "+zksh" "-target-feature" "+svinval"
+// MCPU-XIANGSHAN-NANHU: "-target-abi" "lp64d"
+
 // We cannot check much for -mcpu=native, but it should be replaced by a valid CPU string.
 // RUN: %clang --target=riscv64 -### -c %s -mcpu=native 2> %t.err || true
 // RUN: FileCheck --input-file=%t.err -check-prefix=MCPU-NATIVE %s
@@ -62,6 +73,9 @@
 // RUN: %clang --target=riscv64 -### -c %s 2>&1 -mtune=veyron-v1 | FileCheck -check-prefix=MTUNE-VEYRON-V1 %s
 // MTUNE-VEYRON-V1: "-tune-cpu" "veyron-v1"
 
+// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mtune=xiangshan-nanhu | FileCheck -check-prefix=MTUNE-XIANGSHAN-NANHU %s
+// MTUNE-XIANGSHAN-NANHU: "-tune-cpu" "xiangshan-nanhu"
+
 // Check mtune alias CPU has resolved to the right CPU according XLEN.
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mtune=generic | FileCheck -check-prefix=MTUNE-GENERIC-32 %s
 // MTUNE-GENERIC-32: "-tune-cpu" "generic"
diff --git a/clang/test/Misc/target-invalid-cpu-note.c b/clang/test/Misc/target-invalid-cpu-note.c
index b2a04ebdbce628f..8e91eb4c62dd259 100644
--- a/clang/test/Misc/target-invalid-cpu-note.c
+++ b/clang/test/Misc/target-invalid-cpu-note.c
@@ -85,7 +85,7 @@
 
 // RUN: not %clang_cc1 -triple riscv64 -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix RISCV64
 // RISCV64: error: unknown target CPU 'not-a-cpu'
-// RISCV64-NEXT: note: valid target CPU values are: generic-rv64, rocket-rv64, sifive-s21, sifive-s51, sifive-s54, sifive-s76, sifive-u54, sifive-u74, sifive-x280, veyron-v1{{$}}
+// RISCV64-NEXT: note: valid target CPU values are: generic-rv64, rocket-rv64, sifive-s21, sifive-s51, sifive-s54, sifive-s76, sifive-u54, sifive-u74, sifive-x280, veyron-v1, xiangshan-nanhu{{$}}
 
 // RUN: not %clang_cc1 -triple riscv32 -tune-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix TUNE-RISCV32
 // TUNE-RISCV32: error: unknown target CPU 'not-a-cpu'
@@ -93,4 +93,4 @@
 
 // RUN: not %clang_cc1 -triple riscv64 -tune-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix TUNE-RISCV64
 // TUNE-RISCV64: error: unknown target CPU 'not-a-cpu'
-// TUNE-RISCV64-NEXT: note: valid target CPU values are: generic-rv64, rocket-rv64, sifive-s21, sifive-s51, sifive-s54, sifive-s76, sifive-u54, sifive-u74, sifive-x280, veyron-v1, generic, rocket, sifive-7-series{{$}}
+// TUNE-RISCV64-NEXT: note: valid target CPU values are: generic-rv64, rocket-rv64, sifive-s21, sifive-s51, sifive-s54, sifive-s76, sifive-u54, sifive-u74, sifive-x280, veyron-v1, xiangshan-nanhu, generic, rocket, sifive-7-series{{$}}
diff --git a/llvm/lib/Target/RISCV/RISCVProcessors.td b/llvm/lib/Target/RISCV/RISCVProcessors.td
index e4008d145ffa572..427e15bea2f8dce 100644
--- a/llvm/lib/Target/RISCV/RISCVProcessors.td
+++ b/llvm/lib/Target/RISCV/RISCVProcessors.td
@@ -243,3 +243,24 @@ def VENTANA_VEYRON_V1 : RISCVProcessorModel<"veyron-v1",
                                              FeatureStdExtZicbop,
                                              FeatureStdExtZicboz,
                                              FeatureVendorXVentanaCondOps]>;
+
+def XIANGSHAN_NANHU : RISCVProcessorModel<"xiangshan-nanhu",
+                                          NoSchedModel,
+                                          [Feature64Bit,
+                                           FeatureStdExtZicsr,
+                                           FeatureStdExtZifencei,
+                                           FeatureStdExtM,
+                                           FeatureStdExtA,
+                                           FeatureStdExtF,
+                                           FeatureStdExtD,
+                                           FeatureStdExtC,
+                                           FeatureStdExtZba,
+                                           FeatureStdExtZbb,
+                                           FeatureStdExtZbc,
+                                           FeatureStdExtZbs,
+                                           FeatureStdExtZkn,
+                                           FeatureStdExtZksed,
+                                           FeatureStdExtZksh,
+                                           FeatureStdExtSvinval,
+                                           FeatureStdExtZicbom,
+                                           FeatureStdExtZicboz]>;

@wangpc-pp
Copy link
Contributor

LGTM in general, except one question: will zicbom and zicboz be in the final RTL?

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Oct 26, 2023

LGTM in general, except one question: will zicbom and zicboz be in the final RTL?

You can find the full implementation of zicbom and zicboz here: OpenXiangShan/XiangShan@ca18a0b.
cc @poemonsense

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -20,6 +20,17 @@
// MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
// MCPU-SYNTACORE-SCR1-MAX: "-target-abi" "ilp32"

// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=xiangshan-nanhu | FileCheck -check-prefix=MCPU-XIANGSHAN-NANHU %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to one CHECK line for one feature, but it's OK since other processors aren't in this form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes checking that there is no unrelated feature is important. linux-cross.cpp // DEBIAN_I686_M64-SAME: {{^}} provides an example. But here it may not be important.

@wangpc-pp
Copy link
Contributor

LGTM in general, except one question: will zicbom and zicboz be in the final RTL?

You can find the full implementation of zicbom and zicboz here: OpenXiangShan/XiangShan@ca18a0b. cc @poemonsense

Please update the official document then. :-)

@asb
Copy link
Contributor

asb commented Oct 26, 2023

@dtcxzyw Could you please confirm the status of this core - is it commercially available, an academic test chip, something else?

@poemonsense
Copy link

LGTM in general, except one question: will zicbom and zicboz be in the final RTL?

Yes, it's in the final RTL.

@poemonsense
Copy link

@dtcxzyw Could you please confirm the status of this core - is it commercially available, an academic test chip, something else?

It's maintained by Beijing Institute of Open Source Chip (BOSC), a non-profit organziation founded by companies and researech institutions. It is provided as an open-source CPU Core IP to the third party. It's not an academic testchip.

It's not commercially available now, but there have already been a couple of companies using it as the CPU Core IP. I cannot disclose the progress of commercial chips now but hopefully we will see it soon. There are also some companies using it on FPGAs now.

@poemonsense
Copy link

Please update the official document then. :-)

I'll do it. We did not list it previously because it is not the usual thing for user programs, as we were initially implementing it for internal usage. Sorry for causing the confusion.

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Oct 26, 2023

Any more questions about XiangShan? If there is no question, I will merge this PR tomorrow.

@dtcxzyw dtcxzyw requested a review from asb October 26, 2023 17:00
@@ -20,6 +20,17 @@
// MCPU-SYNTACORE-SCR1-MAX: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
// MCPU-SYNTACORE-SCR1-MAX: "-target-abi" "ilp32"

// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=xiangshan-nanhu | FileCheck -check-prefix=MCPU-XIANGSHAN-NANHU %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes checking that there is no unrelated feature is important. linux-cross.cpp // DEBIAN_I686_M64-SAME: {{^}} provides an example. But here it may not be important.

@asb
Copy link
Contributor

asb commented Oct 27, 2023

@dtcxzyw Could you please confirm the status of this core - is it commercially available, an academic test chip, something else?

It's maintained by Beijing Institute of Open Source Chip (BOSC), a non-profit organziation founded by companies and researech institutions. It is provided as an open-source CPU Core IP to the third party. It's not an academic testchip.

It's not commercially available now, but there have already been a couple of companies using it as the CPU Core IP. I cannot disclose the progress of commercial chips now but hopefully we will see it soon. There are also some companies using it on FPGAs now.

Thanks for clarifying. We don't really have an established policy here - it's obvious that commercial designs with active support should go in, and that some core design I hacked up over a weekend shouldn't but we haven't had the need to discuss anything in-between that. This patch and the scheduling model aren't large, but if there's thought of adding substantially more specific tuning it might be worth discussing those plans.

All that said, @dtcxzyw this is obviously far from your first patch so I have no concern about support being maintained. @preames suggested that we might want to think about deprecation policy so that we can be fairly liberal in accepting support for new CPUs/microarchs, yet remove them later if they become less relevant. Before you go ahead with the merge, it would be good to confirm that @preames agrees that this patch doesn't need to block on discussing that deprecation policy.

Thanks!

@wangpc-pp
Copy link
Contributor

Xiangshan is of great famousness in China and there is already a community in which many individual developers and organiztions/companies like PLCT, T-Head have participated. So I think we needn't worry about the maintenance. :-)

@asb
Copy link
Contributor

asb commented Oct 27, 2023

Xiangshan is of great famousness in China and there is already a community in which many individual developers and organiztions/companies like PLCT, T-Head have participated. So I think we needn't worry about the maintenance. :-)

Thanks for that extra context!

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Nov 1, 2023

Ping. @preames Any more comments?

Copy link
Contributor

@asb asb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everyone is happy, so please go ahead and squash+merge. Thanks!

@dtcxzyw dtcxzyw merged commit 867ece1 into llvm:main Nov 7, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants