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 sifive-p450 CPU. #75760

Merged
merged 3 commits into from
Dec 20, 2023
Merged

[RISCV] Add sifive-p450 CPU. #75760

merged 3 commits into from
Dec 20, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Dec 18, 2023

This is an out of order core with no vector unit. More information: https://www.sifive.com/cores/performance-p450-470

Scheduler model and other tuning will come in separate patches.

This is an out of order core with no vector unit. More information:
https://www.sifive.com/cores/performance-p450-470

Scheduler model and other tuning will come in separate patches.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V labels Dec 18, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 18, 2023

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

@llvm/pr-subscribers-clang

Author: Craig Topper (topperc)

Changes

This is an out of order core with no vector unit. More information: https://www.sifive.com/cores/performance-p450-470

Scheduler model and other tuning will come in separate patches.


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

2 Files Affected:

  • (modified) clang/test/Misc/target-invalid-cpu-note.c (+2-2)
  • (modified) llvm/lib/Target/RISCV/RISCVProcessors.td (+19)
diff --git a/clang/test/Misc/target-invalid-cpu-note.c b/clang/test/Misc/target-invalid-cpu-note.c
index e840a9208f5a45..48e9f05d9b03de 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, xiangshan-nanhu{{$}}
+// RISCV64-NEXT: note: valid target CPU values are: generic-rv64, rocket-rv64, sifive-p450, 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, xiangshan-nanhu, generic, rocket, sifive-7-series{{$}}
+// TUNE-RISCV64-NEXT: note: valid target CPU values are: generic-rv64, rocket-rv64, sifive-p450, 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 58989fd716fa0e..16c79519fcacc1 100644
--- a/llvm/lib/Target/RISCV/RISCVProcessors.td
+++ b/llvm/lib/Target/RISCV/RISCVProcessors.td
@@ -216,6 +216,25 @@ def SIFIVE_X280 : RISCVProcessorModel<"sifive-x280", SiFive7Model,
                                       [TuneSiFive7,
                                        TuneDLenFactor2]>;
 
+def SIFIVE_P450 : RISCVProcessorModel<"sifive-p450", NoSchedModel,
+                                      [Feature64Bit,
+                                       FeatureStdExtZifencei,
+                                       FeatureStdExtM,
+                                       FeatureStdExtA,
+                                       FeatureStdExtF,
+                                       FeatureStdExtD,
+                                       FeatureStdExtC,
+                                       FeatureStdExtZicbop,
+                                       FeatureStdExtZicbom,
+                                       FeatureStdExtZicboz,
+                                       FeatureStdExtZihintntl,
+                                       FeatureStdExtZihintpause,
+                                       FeatureStdExtZihpm,
+                                       FeatureStdExtZba,
+                                       FeatureStdExtZbb,
+                                       FeatureStdExtZbs,
+                                       FeatureStdExtZfhmin]>;
+
 def SYNTACORE_SCR1_BASE : RISCVProcessorModel<"syntacore-scr1-base",
                                               SyntacoreSCR1Model,
                                               [Feature32Bit,

@wangpc-pp
Copy link
Contributor

Add test in clang/test/Driver/riscv-cpus.c?

@tclin914 tclin914 self-requested a review December 19, 2023 07:33
@llvmbot llvmbot added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Dec 20, 2023
@@ -222,6 +222,11 @@
// MCPU-SIFIVE-X280-SAME: "-target-feature" "+zvl512b" "-target-feature" "+zvl64b"
// MCPU-SIFIVE-X280-SAME: "-target-abi" "lp64d"

// RUN: %clang -target riscv64 -### -c %s 2>&1 -menable-experimental-extensions -mcpu=sifive-p450 | FileCheck -check-prefix=MCPU-SIFIVE-P450 %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any experimental extensions?

@@ -222,6 +222,11 @@
// MCPU-SIFIVE-X280-SAME: "-target-feature" "+zvl512b" "-target-feature" "+zvl64b"
// MCPU-SIFIVE-X280-SAME: "-target-abi" "lp64d"

// RUN: %clang -target riscv64 -### -c %s 2>&1 -menable-experimental-extensions -mcpu=sifive-p450 | FileCheck -check-prefix=MCPU-SIFIVE-P450 %s
// MCPU-SIFIVE-P450: "-nostdsysteminc" "-target-cpu" "sifive-p450"
// MCPU-SIFIVE-P450-SAME: "-target-feature" "+m" "-target-feature" "+a" "-target-feature" "+f" "-target-feature" "+d" "-target-feature" "+c" "-target-feature" "+zicbom" "-target-feature" "+zicbop" "-target-feature" "+zicboz" "-target-feature" "+zicsr" "-target-feature" "+zifencei" "-target-feature" "+zihintntl" "-target-feature" "+zihintpause" "-target-feature" "+zihpm" "-target-feature" "+zfhmin" "-target-feature" "+zba" "-target-feature" "+zbb" "-target-feature" "+zbs" "-target-feature" "-
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this line end with "-? Copy/paste mistake?
And this line is too long, please add some line breaks.

Copy link
Collaborator Author

@topperc topperc Dec 20, 2023

Choose a reason for hiding this comment

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

The "- is to make sure we check every positive feature. There are only negative features from there on.

I don't know how to add line breaks while making sure there are no target-features in the output that aren't being checked. The -SAME checks used on other CPUs allow things to be missed. For example SIFIVE-X280 should have +zfhmin after the recent change to make Zfh imply Zfhmin, but it's not caught because it occurs in the gap between the line that ends in +zfh and the next line.

I'm open to other suggestions for how to do this.

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 the form of veyron-v1 test that there is exactly one line for one feature. We can just test features specified in RISCVProcessorModel definitions in RISCVProcessors.td. As for implied features, we can ignore them as there are test coverage in other tests and this is not the purpose of riscv-cpus.c I think.

FeatureStdExtZbb,
FeatureStdExtZbs,
FeatureStdExtZfhmin]>;

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't find the supported ISA description in datasheet, so I wonder if it's true that only Zfhmin is implemented. Should it be Zfh?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Zfhmin is correct. Its the minimum required for RVA22/RVA23 profiles.

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.

@topperc topperc merged commit b03f0c5 into llvm:main Dec 20, 2023
4 checks passed
@topperc topperc deleted the pr/p450-name branch December 20, 2023 17:52
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

3 participants