Skip to content

Add RISC-V support information to readme #132699

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 31, 2025
Merged

Add RISC-V support information to readme #132699

merged 1 commit into from
Mar 31, 2025

Conversation

AnastasiyaChernikova
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Mar 24, 2025 •
edited
Loading

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-tools-llvm-exegesis

Author: None (AnastasiyaChernikova)

Changes

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

1 Files Affected:

  • (modified) llvm/tools/llvm-exegesis/README.md (+2)
diff --git a/llvm/tools/llvm-exegesis/README.md b/llvm/tools/llvm-exegesis/README.md
index deb0f230f032f..b58a9bcaa2cf2 100644
--- a/llvm/tools/llvm-exegesis/README.md
+++ b/llvm/tools/llvm-exegesis/README.md
@@ -32,6 +32,8 @@ architectures:
     e.g. pseudo instructions and most register classes are not supported.
 * MIPS
 * PowerPC (PowerPC64LE only)
+* RISCV
+  * Supported extensions: compressed, atomic, multiply-divide, initual vector instructions.
 
 Note that not all benchmarking functionality is guaranteed to work on all platforms.

@r-belenov
Copy link
Contributor

Typo initual/initial )

@AnastasiyaChernikova AnastasiyaChernikova self-assigned this Mar 24, 2025
boomanaiden154
boomanaiden154 reviewed Mar 24, 2025
View reviewed changes
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

I'm assuming this is riscv64 only?

mshockwave
mshockwave reviewed Mar 24, 2025
View reviewed changes
Copy link
Member

@mshockwave mshockwave left a comment

Choose a reason for hiding this comment

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

Please also use a proper title (i.e. "[Exegesis]...") and add a description.

llvm/tools/llvm-exegesis/README.md Outdated
@@ -32,6 +32,8 @@ architectures:
e.g. pseudo instructions and most register classes are not supported.
* MIPS
* PowerPC (PowerPC64LE only)
* RISCV
* Supported extensions: compressed, atomic, multiply-divide, initial vector instructions.
Copy link
Member

Choose a reason for hiding this comment

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

could you use the formal extension names (e.g. C, A, M, ...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Scalar FP and bitmanip work don't they? I've tested them on HiFive Premier P550

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

topperc
topperc reviewed Mar 25, 2025
View reviewed changes
llvm/tools/llvm-exegesis/README.md Outdated
@@ -32,6 +32,8 @@ architectures:
e.g. pseudo instructions and most register classes are not supported.
* MIPS
* PowerPC (PowerPC64LE only)
* RISCV
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the official spelling RISC-V

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

@AnastasiyaChernikova
Copy link
Contributor Author

I'm assuming this is riscv64 only?

As I understand, also for riscv32. Many teams are gathering under this architecture.

@boomanaiden154
Copy link
Contributor

As I understand, also for riscv32. Many teams are gathering under this architecture.

Can you add a line saying both riscv64 and riscv32 are supported?

@r-belenov
Copy link
Contributor

Please also update https://github.com/llvm/llvm-project/blob/main/llvm/docs/CommandGuide/llvm-exegesis.rst

topperc
topperc reviewed Mar 26, 2025
View reviewed changes
llvm/tools/llvm-exegesis/README.md Outdated
@@ -32,6 +32,8 @@ architectures:
e.g. pseudo instructions and most register classes are not supported.
* MIPS
* PowerPC (PowerPC64LE only)
* RISC-V
* Supported extensions: M, A, F, C, B, initial V.
Copy link
Collaborator

Choose a reason for hiding this comment

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

D?

Copy link
Member

Choose a reason for hiding this comment

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

Reading this part again, I don't think we explicitly exclude any extensions, right? It's usually technical shortfalls that limit the kind of instructions we can measure / generate (e.g. we still can't measure memory instructions in RVV). So perhaps we can rephrase this part of the description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the wording, please take a look

boomanaiden154
boomanaiden154 approved these changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

Nits, otherwise LGTM.

Please wait for Craig/Min to approve as well before merging though.

llvm/docs/CommandGuide/llvm-exegesis.rst Outdated
(AArch64 only, snippet generation is sparse), MIPS, and PowerPC (PowerPC64LE
only) on Linux for benchmarking. Not all benchmarking functionality is
(AArch64 only, snippet generation is sparse), MIPS, PowerPC (PowerPC64LE
only) and RISC-V (RV64I/E and RV32I/E) on Linux for benchmarking. Not all benchmarking functionality is
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you wrap this line and maybe reflow the text around it t 80 chars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

llvm/tools/llvm-exegesis/README.md Outdated
@@ -32,6 +32,8 @@ architectures:
e.g. pseudo instructions and most register classes are not supported.
* MIPS
* PowerPC (PowerPC64LE only)
* RISC-V
* RV64I/E, RV32I/E and extensions supported by LLVM's RISC-V backend with some limitations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

@AnastasiyaChernikova
Copy link
Contributor Author

@mshockwave @topperc Please take a look and see if there are any comments?

topperc
topperc approved these changes Mar 31, 2025
View reviewed changes
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@AnastasiyaChernikova AnastasiyaChernikova merged commit 5a30794 into llvm:main Mar 31, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mshockwave mshockwave mshockwave approved these changes

@topperc topperc topperc approved these changes

@boomanaiden154 boomanaiden154 boomanaiden154 approved these changes

Assignees

@AnastasiyaChernikova AnastasiyaChernikova

Labels
llvm:binary-utilities tools:llvm-exegesis
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AnastasiyaChernikova @llvmbot @r-belenov @boomanaiden154 @mshockwave @topperc