Skip to content

Conversation

@Sukuna0007Abhi
Copy link
Contributor

@Sukuna0007Abhi Sukuna0007Abhi commented Jul 23, 2025

Add AMO Instruction Layout System for Zaamo Extension

fixes #223

Supports #361 and #508

This PR implements a clean layout-based system for generating Atomic Memory Operation (AMO) instructions for the Zaamo extension, combining approaches suggested by @ddrcode and @ThinkOpenly.

✨ What's New
Layout Templates: Created 9 ERB templates (one per AMO operation) that automatically generate both word and doubleword variants
Organized Structure: Added layouts/ folder for templates and instructions/ folder for generated files
Automated Generation: Updated Rakefile to generate all 18 AMO instruction files from the layout templates
Better Descriptions: Each instruction now has proper long names and bullet-point descriptions
Schema Fixes: Fixed Zaamo.yaml extension definition with correct schema path

🎯 Result
Instead of manually maintaining 18 separate instruction files, we now have:

9 maintainable layout templates
Automatic generation of all AMO variants (.w and .d)
Consistent formatting and descriptions across all instructions
Easy to add new AMO operations in the future

🧹 Cleanup

Also updated .gitignore to exclude the 270+ generated CSR test files that were cluttering the repo.

Ready for review! 🎉 @AFOliveira @ThinkOpenly @dhower-qc

Implement layout templates for all 9 AMO operations with organized folders.
Create Rakefile entries to generate 18 instruction variants from layouts.

Resolves riscv-software-src#223

Signed-off-by: GitHub <[email protected]>
@ThinkOpenly
Copy link
Collaborator

Add AMO Instruction Layout System for Zaamo Extension

fixes #223

Thanks for your efforts!

Layout Templates: Created 9 ERB templates (one per AMO operation) that automatically generate both word and doubleword variants

Sounds good, except that #223 is for Byte and Halfword atomics.

Organized Structure: Added layouts/ folder for templates and instructions/ folder for generated files

I understand why you thought to do this, but this is not how we've been doing it so far. 🙂 The file extensions are sufficient to distinguish between layouts and instruction YAML. Let's leave everything where it is for now.

Automated Generation: Updated Rakefile to generate all 18 AMO instruction files from the layout templates

OK

🎯 Result Instead of manually maintaining 18 separate instruction files, we now have:

9 maintainable layout templates Automatic generation of all AMO variants (.w and .d)

This is nice. This is also a step towards #361. Take a peek at that and see if that's something you wish to also include in this PR. Basically, all of these AMO instructions have not only the "width" variants, but also the "acquire/release" variants. For example, amoadd ends up with:

  • amoadd.b, amoadd.b.aq, amoadd.b.rl, amoadd.b.aq.rl
  • amoadd.h, amoadd.h.aq, amoadd.h.rl, amoadd.h.aq.rl
  • amoadd.w, amoadd.w.aq, amoadd.w.rl, amoadd.w.aq.rl
  • amoadd.d, amoadd.d.aq, amoadd.d.rl, amoadd.d.aq.rl

Also updated .gitignore to exclude the 270+ generated CSR test files that were cluttering the repo.

These are files that we want to keep. The general process for "layout" files is to commit both the layout file and the generated files.

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Jul 24, 2025

Add AMO Instruction Layout System for Zaamo Extension
fixes #223

Thanks for your efforts!

Thanks

Layout Templates: Created 9 ERB templates (one per AMO operation) that automatically generate both word and doubleword variants

Sounds good, except that #223 is for Byte and Halfword atomics.

Organized Structure: Added layouts/ folder for templates and instructions/ folder for generated files

I understand why you thought to do this, but this is not how we've been doing it so far. 🙂 The file extensions are sufficient to distinguish between layouts and instruction YAML. Let's leave everything where it is for now.

**Yes sir I thought the both layout flies and the instruction YAML will be mixed up that's but ok sir I will change that by today and should I leave the layout folder as it is and only deleting instruction folder. @ThinkOpenly **

Automated Generation: Updated Rakefile to generate all 18 AMO instruction files from the layout templates

OK

🎯 Result Instead of manually maintaining 18 separate instruction files, we now have:
9 maintainable layout templates Automatic generation of all AMO variants (.w and .d)

This is nice. This is also a step towards #361. Take a peek at that and see if that's something you wish to also include in this PR. Basically, all of these AMO instructions have not only the "width" variants, but also the "acquire/release" variants. For example, amoadd ends up with:

  • amoadd.b, amoadd.b.aq, amoadd.b.rl, amoadd.b.aq.rl
  • amoadd.h, amoadd.h.aq, amoadd.h.rl, amoadd.h.aq.rl
  • amoadd.w, amoadd.w.aq, amoadd.w.rl, amoadd.w.aq.rl
  • amoadd.d, amoadd.d.aq, amoadd.d.rl, amoadd.d.aq.rl

Ok sir will definitely work on it

Also updated .gitignore to exclude the 270+ generated CSR test files that were cluttering the repo.

These are files that we want to keep. The general process for "layout" files is to commit both the layout file and the generated files.

Ok then I will change the gitignore then both files will be there

Implement layout templates for 9 AMO operations to generate word and doubleword variants.
Update Rakefile to generate AMO instructions from layout templates.
Add organized structure with proper descriptions following project conventions.

Resolves riscv-software-src#223

Signed-off-by: GitHub <[email protected]>
Add 9 dynamic layout files generating 144 instruction variants
(9 ops × 4 sizes × 4 aq/rl combinations) with automated Rakefile
generation, replacing manual YAML maintenance per issue riscv-software-src#361.

Resolves: riscv-software-src#361
Signed-off-by: GitHub <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Jul 25, 2025

Hi @ThinkOpenly, @dhower-qc @AFOliveira

I've implemented the AMO layout system with your byte/halfword approach (it will now work for 4 variants b, h w, d) and issue #361's acquire/release variants. The 9 layout files now generate all 144 instruction combinations (base, .aq, .rl, .aq.rl) automatically, replacing manual YAML maintenance.

And also did the gitignore part and rewrite the folder structure

Your conditional descriptions for "least-significant X of register" vs "value of register" are working perfectly across all operations.

Ready for Re-review! 🚀

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Jul 25, 2025

And @ThinkOpenly @dhower-qc @AFOliveira from the issue #361 I implemented the solution by these steps:
Issue #361 Implementation:

Added acquire/release memory ordering variants to AMO instructions:

Generate 4 variants per operation: base, .aq, .rl, .aq.rl
Fixed bit encoding (hardcoded aq/rl bits instead of variables)
Dynamic instruction naming and descriptions based on memory ordering flags
Complete 144 instruction coverage (9 ops × 4 sizes × 4 aq/rl variants)

Ready for review. @ThinkOpenly @dhower-qc

@jordancarlin
Copy link
Contributor

I've implemented the AMO layout system with your byte/halfword approach (it will now work for 4 variants b, h w, d) and issue #361's acquire/release variants. The 9 layout files now generate all 144 instruction combinations (base, .aq, .rl, .aq.rl) automatically, replacing manual YAML maintenance.

Minor but important correction: the four suffixes should be nothing, .aq, .rl, and .aqrl (not .aq.rl).

@ThinkOpenly
Copy link
Collaborator

Minor but important correction: the four suffixes should be nothing, .aq, .rl, and .aqrl (not .aq.rl).

Oops, I messed that up. Trust, but verify.

@Sukuna0007Abhi , could you update this PR to reflect that correction?

@Sukuna0007Abhi
Copy link
Contributor Author

Sure,updating by today @ThinkOpenly
& Thanks for suggesting @jordancarlin

…MO variants

- Update all Zaamo AMO layout files and Rakefile to use .aqrl suffix for aq+rl
- Ensures compliance with reviewer feedback and RISC-V AMO naming conventions

Signed-off-by: GitHub <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Jul 27, 2025

hi sir @ThinkOpenly and @jordancarlin I updated that and now all suffixes will follow the correct convention: .aqrl (not .aq.rl) for combined aq/rl. Thanks for catching this! Ready for re-review. @ThinkOpenly @dhower-qc

- Update  reference from ../../../.. to ../../.. in Zaamo AMO layouts
- Ensures correct schema resolution for instruction files

Signed-off-by: GitHub <[email protected]>
@ThinkOpenly
Copy link
Collaborator

@Sukuna0007Abhi, could you check why this PR has 272 file changes? I think there is a lot of stuff here that shouldn't be here.

Also, there are some files missing: when we add layout generation, we commit the generated files here as well. So, while I certainly expect the .layout files in spec/std/isa/inst/Zaamo, I also want to see the generated .yaml files:

Complete 144 instruction coverage (9 ops × 4 sizes × 4 aq/rl variants)

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Jul 28, 2025

Sir @ThinkOpenly those are the generated yaml files before you told for the 4 variants adding part.

Those are actually before the changes of variants and b/h and gitignore update, this is from the first commit of this PR, at beginning I thought no need to commit those generated files, so I deleted them but then found those are also was before in this repo so that's why it is showing this I will again regenerate and now push all yaml files also.

dhower-qc and others added 10 commits July 29, 2025 21:08
…c#919)

This should let us make sure PRs fail in the merge queue if they fail to
build a deployment target
…ware-src#908)

- Fixes the generation of an extension PDF when a custom overlay is
involved
- Fixes some minor IDL issues in Xqci that were revealed by recent
compiler improvements
- Adds the Xqci document generation to regression tests
…cv-software-src#920)

Take 2; the first attempt didn't work because the 'dry-run' input isn't
in scope when triggered from the merge queue.
Implement layout templates for 9 AMO operations to generate word and doubleword variants.
Update Rakefile to generate AMO instructions from layout templates.
Add organized structure with proper descriptions following project conventions.

Resolves riscv-software-src#223

Signed-off-by: GitHub <[email protected]>
Add 9 dynamic layout files generating 144 instruction variants
(9 ops × 4 sizes × 4 aq/rl combinations) with automated Rakefile
generation, replacing manual YAML maintenance per issue riscv-software-src#361.

Resolves: riscv-software-src#361
Signed-off-by: GitHub <[email protected]>
Added bit 16 (Q-bit) to `sw_read()` for `misa` CSR according to Table 10
in Section 3.1.1. of The RISC-V Instruction Set Manual Volume II version
20250508.
… conditional (riscv-software-src#911)

Instruction AsciiDoc templates always display access modes for M, S, and
U modes (and VS, VU when H extension is present), even when U/S modes
aren't implemented in the system.

- Modify instruction AsciiDoc templates to conditionally display only
the privilege modes that are actually implemented
- Check for S, U, and H extension presence before showing corresponding
access modes
- Always display M-mode as it's always present in RISC-V systems
- Show S-mode (or HS-mode when H extension is present) only when S
extension is implemented
- Show U-mode only when U extension is implemented
- Show VS/VU modes only when H extension is implemented

---------

Co-authored-by: 7908837174 <[email protected]>
…MO variants

- Update all Zaamo AMO layout files and Rakefile to use .aqrl suffix for aq+rl
- Ensures compliance with reviewer feedback and RISC-V AMO naming conventions

Signed-off-by: GitHub <[email protected]>
- Update  reference from ../../../.. to ../../.. in Zaamo AMO layouts
- Ensures correct schema resolution for instruction files

Signed-off-by: GitHub <[email protected]>
- Fix AMONone to AmoNone in all AMOCAS function implementations
- Resolves compilation errors: 'AMONone' is not a member of 'udb::PmaAttribute'
- Should fix regress-riscv-tests CI check failures
- All 20+ error instances stem from this single enum naming issue

Signed-off-by: GitHub <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

ok fianlly after researching some days and working now I think it's done all cl checks now also passing , I also added the amocas part added amocas function part in globals.isa and it's layout in Zabha but it still needs update, for it I need more info about Zacas , which I will do in different PR, and updated the opcodes updated rakefile for generation of amo files and updated layout files to properly handles all variants. Ready for Review @ThinkOpenly @dhower-qc @AFOliveira

- Change wavedrom path from /workspaces/ back to /home/hbg/Projects/
- Maintains consistency with upstream repository format
- Prevents environment-specific paths from being merged to main repo

Signed-off-by: GitHub <[email protected]>
@codecov
Copy link

codecov bot commented Aug 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.05%. Comparing base (5c9aa28) to head (267b75f).
⚠️ Report is 37 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #921   +/-   ##
=======================================
  Coverage   46.05%   46.05%           
=======================================
  Files          11       11           
  Lines        4942     4942           
  Branches     1345     1345           
=======================================
  Hits         2276     2276           
  Misses       2666     2666           
Flag Coverage Δ
idlc 46.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Sukuna0007Abhi
Copy link
Contributor Author

Plz review sir @ThinkOpenly @dhower-qc @AFOliveira

@Sukuna0007Abhi Sukuna0007Abhi changed the title feat: add amo instruction layout system for Zaamo & Zabha extension feat: add amo instruction layout system for Zaamo & Zabha extension & some Zacas extension Aug 21, 2025
This commit refactors the Rakefile build system and fixes AMO instruction
layout templates to eliminate code duplication and enable proper
dependency-driven generation:

**Rakefile Changes:**
- Replace manual AMO file generation with dependency-driven system
- Remove ~50 lines of duplicate manual task invocations
- Add AMO instruction file dependencies to test:inst_encodings task
- Generate 152 AMO files automatically through proper dependencies

**Layout Template Fixes:**
- spec/std/isa/inst/Zaamo/amomaxu.SIZE.AQRL.layout: Remove duplicate content
- spec/std/isa/inst/Zabha/amocas.SIZE.AQRL.layout: Fix templated function call syntax
  (amocas<<%= ... %>> → amocas<%= ... %>)

**Benefits:**
- Eliminates mentor-flagged code duplication in Rakefile
- Enables automatic AMO instruction file generation through dependencies
- Fixes template syntax errors preventing proper file generation
- Maintains full functionality while improving maintainability

The dependency system ensures AMO instruction files are generated
automatically when needed, removing the need for manual task invocations
while preserving all existing functionality.

Signed-off-by: GitHub <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

pls check sir @ThinkOpenly sir @dhower-qc sir @AFOliveira

…sions

- Add AMO instruction generation rules in Rakefile for automated file creation
- Implement layout-based generation for all AMO operations (amoadd, amoand, amomax, etc.)
- Support all acquire/release combinations (.aq, .rl, .aqrl variants)
- Generate Zaamo instructions for word/doubleword sizes (w/d)
- Generate Zabha instructions for byte/halfword sizes (b/h) plus amocas variants
- Add proper auto-generation warnings with insert_warning function
- Organize generation in gen namespace for proper separation from tests
- Include complete Sail model implementations for all generated instructions
- Fix whitespace issues and apply prettier formatting

Addresses mentor feedback on namespace organization and missing auto-generation warnings.

Signed-off-by: GitHub <[email protected]>
Update the golden reference file to include the newly added AMO instructions
for Zaamo and Zabha extensions generated by the layout system.

Signed-off-by: GitHub <[email protected]>
Synchronize golden file with the current instruction appendix generation
that includes all AMO instructions for Zaamo, Zabha, and existing Zacas extensions.

Signed-off-by: GitHub <[email protected]>
…on layout system

- Remove commented Zacas code from Rakefile to clean up build system
- Fix corrupted long_name/description fields in AMO layout templates by converting multi-line ERB to single-line
- Add missing 'fetch-and-' prefixes for amoand and amoor instructions
- Correct sail() method register naming from X(xs2)/X(xd) to X(rs2)/X(rd)
- Add copyright headers to all 10 AMO layout files for REUSE compliance
- Regenerate AMO instruction variant files with corrected content
- Update instruction appendix golden file to reflect proper naming

Addresses feedback from Sir @ThinkOpenly in:
riscv-software-src#921 (review)

All CI checks pass including REUSE compliance and instruction appendix regression tests.

Signed-off-by: GitHub <[email protected]>
@ThinkOpenly
Copy link
Collaborator

I think this only needs one more update to the "golden instruction appendix" to pass the CI tests. Are you willing to do so, @Sukuna0007Abhi ?

@ThinkOpenly ThinkOpenly linked an issue Sep 18, 2025 that may be closed by this pull request
@ThinkOpenly
Copy link
Collaborator

I think this only needs one more update to the "golden instruction appendix" to pass the CI tests. Are you willing to do so, @Sukuna0007Abhi ?

@Sukuna0007Abhi, are you able to add this requested change? If you need someone to take over this PR, please let us know.

@Sukuna0007Abhi
Copy link
Contributor Author

Sorry sir @ThinkOpenly my exams are going on that's why I can't able to contribute now need some time after exams end I will work

@dhower-qc dhower-qc self-assigned this Oct 8, 2025
@dhower-qc
Copy link
Collaborator

ping, are you available to update the golden instruction appendix?

@ThinkOpenly
Copy link
Collaborator

Closing this PR in favor of #1240. I've carried over the changes from here, and preserved the authorship.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Acquire and release instructions have a bug Add Zabha (Byte and Halfword Atomics)

8 participants