https://github.com/llvm/llvm-project/pull/102452 Skip to content Navigation Menu Toggle navigation Sign in * Product + Actions Automate any workflow + Packages Host and manage packages + Security Find and fix vulnerabilities + Codespaces Instant dev environments + GitHub Copilot Write better code with AI + Code review Manage code changes + Issues Plan and track work + Discussions Collaborate outside of code Explore + All features + Documentation + GitHub Skills + Blog * Solutions By size + Enterprise + Teams + Startups By industry + Healthcare + Financial services + Manufacturing By use case + CI/CD & Automation + DevOps + DevSecOps * Resources Topics + AI + DevOps + Security + Software Development + View all Explore + Learning Pathways + White papers, Ebooks, Webinars + Customer Stories + Partners * Open Source + GitHub Sponsors Fund open source developers + The ReadME Project GitHub community articles Repositories + Topics + Trending + Collections * Enterprise + Enterprise platform AI-powered developer platform Available add-ons + Advanced Security Enterprise-grade security features + GitHub Copilot Enterprise-grade AI features + Premium Support Enterprise-grade 24/7 support * Pricing Search or jump to... Search code, repositories, users, issues, pull requests... Search [ ] Clear Search syntax tips Provide feedback We read every piece of feedback, and take your input very seriously. [ ] [ ] Include my email address so I can be contacted Cancel Submit feedback Saved searches Use saved searches to filter your results more quickly Name [ ] Query [ ] To see all available qualifiers, see our documentation. Cancel Create saved search Sign in Sign up Reseting focus You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session. You switched accounts on another tab or window. Reload to refresh your session. Dismiss alert {{ message }} llvm / llvm-project Public * Notifications You must be signed in to change notification settings * Fork 11.3k * Star 27.6k * Code * Issues 5k+ * Pull requests 2.8k * Actions * Security * Insights Additional navigation options * Code * Issues * Pull requests * Actions * Security * Insights 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 [RISCV] Add Hazard3 Core as taped out for RP2350 #102452 Open lenary wants to merge 3 commits into llvm:main base: main Choose a base branch [ ] Branches Tags Could not load branches Branch not found: {{ refName }} Loading {{ refName }} default Could not load tags Nothing to show {{ refName }} default Loading Are you sure you want to change the base? Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated. Loading Change base from lenary:lenary/hazard3 Open [RISCV] Add Hazard3 Core as taped out for RP2350 #102452 lenary wants to merge 3 commits into llvm:main from lenary:lenary/ hazard3 +35 -0 Conversation 23 Commits 3 Checks 5 Files changed 4 Conversation This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters Show hidden characters lenary Copy link Member @lenary lenary commented Aug 8, 2024 * edited Loading Luke Wren's Hazard3 is a configurable, open-source 32-bit RISC-V core. The core's source code and docs are available on github: https://github.com/wren6991/hazard3 This is the RISC-V core used in the RP2350, a recently announced SoC by Raspberry Pi (which also contains Arm cores): https://datasheets.raspberrypi.com/rp2350/rp2350-datasheet.pdf We have agreed to name this -mcpu option rp2350-hazard3, and it reflects exactly the options configured in the RP2350 chips. Notably, the Zbc is not configured, and nor is B because the misa.B bit is not either. Sorry, something went wrong. All reactions @lenary lenary requested review from asb and topperc August 8, 2024 12:00 @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 Aug 8, 2024 @llvmbot Copy link Collaborator llvmbot commented Aug 8, 2024 * edited Loading @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Sam Elliott (lenary) Changes Luke Wren's Hazard3 is an open-source 32-bit RISC-V core. The core's source code and docs are available on github: https://github.com/wren6991/hazard3 The core has just hit 1.0 today, which seems to me to be a good time to add it as a named CPU in LLVM. --------------------------------------------------------------------- Full diff: https://github.com/llvm/llvm-project/pull/102452.diff 4 Files Affected: * (modified) clang/test/Driver/riscv-cpus.c (+17) * (modified) clang/test/Misc/target-invalid-cpu-note.c (+2-2) * (modified) llvm/docs/ReleaseNotes.rst (+1) * (modified) llvm/lib/Target/RISCV/RISCVProcessors.td (+18) diff --git a/clang/test/Driver/riscv-cpus.c b/clang/test/Driver/riscv-cpus.c index 29687ac4e1c83..dd135bb85ea84 100644 --- a/clang/test/Driver/riscv-cpus.c +++ b/clang/test/Driver/riscv-cpus.c @@ -402,6 +402,23 @@ // MCPU-SIFIVE-P670-SAME: "-target-feature" "+zvkt" // MCPU-SIFIVE-P670-SAME: "-target-abi" "lp64d" +// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=hazard3 | FileCheck -check-prefix=MCPU-HAZARD3 %s +// MCPU-HAZARD3: "-target-cpu" "hazard3" +// MCPU-HAZARD3-SAME: "-target-feature" "+m" +// MCPU-HAZARD3-SAME: "-target-feature" "+a" +// MCPU-HAZARD3-SAME: "-target-feature" "+c" +// MCPU-HAZARD3-SAME: "-target-feature" "+b" +// MCPU-HAZARD3-SAME: "-target-feature" "+zicsr" +// MCPU-HAZARD3-SAME: "-target-feature" "+zifencei" +// MCPU-HAZARD3-SAME: "-target-feature" "+zcb" +// MCPU-HAZARD3-SAME: "-target-feature" "+zcmp" +// MCPU-HAZARD3-SAME: "-target-feature" "+zba" +// MCPU-HAZARD3-SAME: "-target-feature" "+zbb" +// MCPU-HAZARD3-SAME: "-target-feature" "+zbc" +// MCPU-HAZARD3-SAME: "-target-feature" "+zbkb" +// MCPU-HAZARD3-SAME: "-target-feature" "+zbs" +// MCPU-HAZARD3-SAME: "-target-abi" "ilp32" + // Check failed cases // RUN: not %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv321 | FileCheck -check-prefix=FAIL-MCPU-NAME %s diff --git a/clang/test/Misc/target-invalid-cpu-note.c b/clang/test/Misc/target-invalid-cpu-note.c index 249bea2311549..7485730296e09 100644 --- a/clang/test/Misc/target-invalid-cpu-note.c +++ b/clang/test/Misc/target-invalid-cpu-note.c @@ -81,7 +81,7 @@ // RUN: not %clang_cc1 -triple riscv32 -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix RISCV32 // RISCV32: error: unknown target CPU 'not-a-cpu' -// RISCV32-NEXT: note: valid target CPU values are: generic-rv32, rocket-rv32, sifive-e20, sifive-e21, sifive-e24, sifive-e31, sifive-e34, sifive-e76, syntacore-scr1-base, syntacore-scr1-max, syntacore-scr3-rv32, syntacore-scr4-rv32{{$}} +// RISCV32-NEXT: note: valid target CPU values are: generic-rv32, hazard3, rocket-rv32, sifive-e20, sifive-e21, sifive-e24, sifive-e31, sifive-e34, sifive-e76, syntacore-scr1-base, syntacore-scr1-max, syntacore-scr3-rv32, syntacore-scr4-rv32{{$}} // 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' @@ -89,7 +89,7 @@ // 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' -// TUNE-RISCV32-NEXT: note: valid target CPU values are: generic-rv32, rocket-rv32, sifive-e20, sifive-e21, sifive-e24, sifive-e31, sifive-e34, sifive-e76, syntacore-scr1-base, syntacore-scr1-max, syntacore-scr3-rv32, syntacore-scr4-rv32, generic, rocket, sifive-7-series{{$}} +// TUNE-RISCV32-NEXT: note: valid target CPU values are: generic-rv32, hazard3, rocket-rv32, sifive-e20, sifive-e21, sifive-e24, sifive-e31, sifive-e34, sifive-e76, syntacore-scr1-base, syntacore-scr1-max, syntacore-scr3-rv32, syntacore-scr4-rv32, generic, rocket, sifive-7-series{{$}} // 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' diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 1ed860de6b9dc..579c1c1a37aab 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -108,6 +108,7 @@ Changes to the RISC-V Backend fill value) rather than NOPs. * Added Syntacore SCR4 CPUs: ``-mcpu=syntacore-scr4-rv32/64`` * ``-mcpu=sifive-p470`` was added. +* Added Hazard3 CPU: ``-mcpu=hazard3`` (32-bit only). Changes to the WebAssembly Backend ---------------------------------- diff --git a/llvm/lib/Target/RISCV/RISCVProcessors.td b/llvm/lib/Target/RISCV/RISCVProcessors.td index ec9322f3e9924..153ad0a661945 100644 --- a/llvm/lib/Target/RISCV/RISCVProcessors.td +++ b/llvm/lib/Target/RISCV/RISCVProcessors.td @@ -428,3 +428,21 @@ def SPACEMIT_X60 : RISCVProcessorModel<"spacemit-x60", FeatureStdExtZvkt, FeatureStdExtZvl256b]), [TuneDLenFactor2]>; + +def HAZARD3 : RISCVProcessorModel<"hazard3", + NoSchedModel, + [Feature32Bit, + FeatureStdExtI, + FeatureStdExtM, + FeatureStdExtA, + FeatureStdExtB, + FeatureStdExtC, + FeatureStdExtZicsr, + FeatureStdExtZifencei, + FeatureStdExtZba, + FeatureStdExtZbb, + FeatureStdExtZbc, + FeatureStdExtZbs, + FeatureStdExtZbkb, + FeatureStdExtZcb, + FeatureStdExtZcmp]>; All reactions Sorry, something went wrong. @lenary lenary marked this pull request as draft August 8, 2024 12:35 @lenary lenary removed request for asb and topperc August 8, 2024 12:49 @dtcxzyw Copy link Member dtcxzyw commented Aug 8, 2024 It looks like a toy project :( The RTL design is not frozen. All reactions Sorry, something went wrong. @michaelmaitland michaelmaitland self-requested a review August 8, 2024 14:20 @michaelmaitland Copy link Contributor michaelmaitland commented Aug 8, 2024 There was a prior discussion about what designs should go in. The initial quote from @asb was: 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 @preames suggested: 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 My main concern here is that we'd like to ensure that there is a maintainer for this. Could you provide us some more information on what entity would be the maintainer? It looks like @Wren6991 is the only maintainer of Hazard3. I'm also interested in understanding whether there are any customers or users of this core. I think it would be helpful to raise this at the next LLVM RISC-V syncup. 2 dtcxzyw and lenary reacted with thumbs up emoji All reactions * 2 reactions Sorry, something went wrong. @asb Copy link Contributor asb commented Aug 8, 2024 FWIW, as of today Hazard3 is in shipping, commercially available silicon https://www.raspberrypi.com/news/ raspberry-pi-pico-2-our-new-5-microcontroller-board-on-sale-now/ 3 dtcxzyw, arturo182, and krasin reacted with thumbs up emoji All reactions * 3 reactions Sorry, something went wrong. @lenary Copy link Member Author lenary commented Aug 8, 2024 I am happy to be point person on maintaining this core definition in LLVM, I communicate regularly with Luke, the core designer. As for the core itself, it's obviously maintained by Luke, and as Alex points out, it's just been announced to be in commercial products released by Raspberry Pi (the RP2350 series of boards, which is the basis for the Pico 2). I realise this means its profile has changed significantly today, away from just being on the hobby end of the open-source core spectrum. 1 krasin reacted with thumbs up emoji All reactions * 1 reaction Sorry, something went wrong. @lenary Copy link Member Author lenary commented Aug 8, 2024 I do have some worries about the definition as-is, as the core in the repo has optional features (configurable at tape-out time). Reading the RP2350 datasheet, not all of the optional features for Hazard3 are enabled (and I'm of the opinion that the lack of arm-like -mcpu= ++no equivalent for RISC-V is a good thing) so I'm not sure exactly what we do here. Maybe a better approach would be to abandon this PR and contribute a -mcpu=raspberrypi-rp2350, which would make it clearer which HDL configuration the compiler option specifically corresponds to? Happy to discuss this next week, thought I'd put some ideas about specific directions down in advance of that. All reactions Sorry, something went wrong. @asb Copy link Contributor asb commented Aug 8, 2024 I do have some worries about the definition as-is, as the core in the repo has optional features (configurable at tape-out time). Reading the RP2350 datasheet, not all of the optional features for Hazard3 are enabled (and I'm of the opinion that the lack of arm-like -mcpu=++no equivalent for RISC-V is a good thing) so I'm not sure exactly what we do here. Maybe a better approach would be to abandon this PR and contribute a -mcpu=raspberrypi-rp2350, which would make it clearer which HDL configuration the compiler option specifically corresponds to? Happy to discuss this next week, thought I'd put some ideas about specific directions down in advance of that. Yeah, I was just looking at the RP2350 datasheet and was about to flag that it indicates no zbc support. All reactions Sorry, something went wrong. @jrtc27 jrtc27 mentioned this pull request Aug 12, 2024 [RISCV] Add NutShell RV32/64 processors definition #102899 Closed @asb Copy link Contributor asb commented Aug 15, 2024 I think our conclusion from the sync-up call discussion matches what we'd roughly concluded in this thread, which is that with an instantiation of Hazard3 shipping in the RP2350 this isn't really a good test case for questions about when it's worth including a CPU definition for an open source core. So for this particular PR, we just need a name people are happy with (perhaps check if the vendor has a preference?). Something like -mcpu =rp2350-hazard3 perhaps? 3 lenary, mshockwave, and Wren6991 reacted with thumbs up emoji All reactions * 3 reactions Sorry, something went wrong. @mshockwave Copy link Member mshockwave commented Aug 15, 2024 So for this particular PR, we just need a name people are happy with (perhaps check if the vendor has a preference?). Something like -mcpu=rp2350-hazard3 perhaps? +1 on -mcpu=rp2350-hazard3 over -mcpu=raspberrypi-rp2350 because the former is more specific to the RISC-V part of RP2350. All reactions Sorry, something went wrong. @lenary Copy link Member Author lenary commented Aug 15, 2024 I've reached out to Luke to ask his/RPi's opinion (but I think he might be away at the moment, so a reply might be slow). My preference remains raspberrypi-rp2350 because it matches our - approach taken so far (for everything except rocket). All reactions Sorry, something went wrong. @Wren6991 Copy link Wren6991 commented Aug 16, 2024 I think we are really compressing a three-tuple of something like --. For a two-tuple I prefer rp2350-hazard3 over raspberrypi-rp2350 because: * RP2350 is a SoC, not a CPU, so doesn't belong at the end of the tuple * RP2350 also has Cortex-M33s, so leaving hazard3 out of the tuple is slightly confusing * Raspberry Pi is the vendor of RP2350 but not the designer of the core * This is specifically for the as-taped-out RP2350 variant of Hazard3 I think rp2350-hazard3 captures this nicely 2 lenary and arturo182 reacted with thumbs up emoji All reactions * 2 reactions Sorry, something went wrong. @lenary lenary force-pushed the lenary/hazard3 branch 2 times, most recently from 043be1e to a874174 Compare August 16, 2024 15:36 @lenary Copy link Member Author lenary commented Aug 16, 2024 I've updated this and gone with rp2350-hazard3 as that's the prevailing consensus. This is stacked on #104601 because i got fed up with the invalid cpu note tests. Otherwise it's ready to review. All reactions Sorry, something went wrong. @lenary lenary marked this pull request as ready for review August 16, 2024 15:36 @lenary lenary changed the title [DEL:[RISCV] Add Hazard3 CPU:DEL] [INS:[RISCV] Add Hazard3 Core as taped out for RP2350:INS] Aug 16, 2024 Wren6991 Wren6991 reviewed Aug 16, 2024 View reviewed changes llvm/docs/ReleaseNotes.rst Outdated @@ -108,6 +108,7 @@ Changes to the RISC-V Backend fill value) rather than NOPs. * Added Syntacore SCR4 and SCR5 CPUs: ``-mcpu=syntacore-scr4/ 5-rv32/64`` * ``-mcpu=sifive-p470`` was added. * Added Hazard3 CPU: ``-mcpu=hazard3`` (32-bit only). Copy link @Wren6991 Wren6991 Aug 16, 2024 There was a problem hiding this comment. Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. [Choose a reason] Hide comment Possibly wants tweaking to match the new flag Sorry, something went wrong. All reactions Copy link Member Author @lenary lenary Aug 16, 2024 There was a problem hiding this comment. Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. [Choose a reason] Hide comment Definitely does. Well spotted, thanks. Sorry, something went wrong. All reactions clang/test/Driver/riscv-cpus.c Show resolved Hide resolved lenary added 2 commits August 19, 2024 07:04 @lenary [clang][test] Split invalid-cpu-note tests ... b125071 This change does two kinds of splits: - Splits each target into a different file. Some targets are left in the same files, such as riscv32/64 and x86/_64 as these tests and lists are very similar. - Splits up the very long 'note:' lines which contain a list of CPUs, using `CHECK-SAME`. There was a note about this not being possible before, but with `--implicit-check-not`, this is now possible -- I have verified that this does the right thing if a single CPU anywhere in the list is left out. These tests had become quite annoying to change when adding a CPU, and I believe this change makes these easier to maintain, and should cut down on conflicts in these files (or at least makes conflicts easier to resolve). I apologise in advance for downstream conflicts, but hopefully that's a small amount of short term pain, in return for fewer conflicts in future. @lenary [RISCV] Add Hazard3 Core as taped out for RP2350 ... 837d32f Luke Wren's Hazard3 is a configurable, open-source 32-bit RISC-V core. The core's source code and docs are available on github: https://github.com/wren6991/hazard3 This is the RISC-V core used in the RP2350, a recently announced SoC by Raspberry Pi (which also contains Arm cores): https://datasheets.raspberrypi.com/rp2350/rp2350-datasheet.pdf We have agreed to name this `-mcpu` option `rp2350-hazard3`, and it reflects exactly the options configured in the RP2350 chips. Notably, the Zbc is not configured, and nor is B because the `misa.B` bit is not either. @lenary lenary force-pushed the lenary/hazard3 branch from a874174 to 837d32f Compare August 19, 2024 14:09 @michaelmaitland Copy link Contributor michaelmaitland commented Aug 19, 2024 * edited Loading Is this PR stacked on b125071 (I don't see any note of that in the PR description at the moment)? Edit: I found the comment that says that! 1 lenary reacted with thumbs up emoji All reactions * 1 reaction Sorry, something went wrong. @lenary Copy link Member Author lenary commented Aug 19, 2024 Is this PR stacked on b125071 (I don't see any note of that in the PR description at the moment)? Edit: I found the comment that says that! Yeah, sorry, I only stacked it about halfway through its life, because I got sick of editing those files. All reactions Sorry, something went wrong. @lenary Merge branch 'main' into lenary/hazard3 531e292 @lenary Copy link Member Author lenary commented Aug 20, 2024 The commit this was stacked on is merged, and I think we have sorted out naming issues and pinned down the exact cpu configuration, so I think this is ready for review and merge. All reactions Sorry, something went wrong. michaelmaitland michaelmaitland approved these changes Aug 20, 2024 View reviewed changes Copy link Contributor @michaelmaitland michaelmaitland left a comment There was a problem hiding this comment. Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. [Choose a reason] Hide comment LGTM Sorry, something went wrong. All reactions Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment Reviewers @Wren6991 Wren6991 Wren6991 left review comments @topperc topperc topperc left review comments @michaelmaitland michaelmaitland michaelmaitland approved these changes Assignees No one assigned 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 Milestone No milestone Development Successfully merging this pull request may close these issues. None yet 8 participants @lenary @llvmbot @dtcxzyw @michaelmaitland @asb @mshockwave @Wren6991 @topperc Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later. Footer (c) 2024 GitHub, Inc. Footer navigation * Terms * Privacy * Security * Status * Docs * Contact * Manage cookies * Do not share my personal information You can't perform that action at this time.