Skip to content

RVVILogger Module to RVVI TEXT format for coverage#1584

Closed
SadhviNarayanan wants to merge 112 commits intoopenhwgroup:mainfrom
SadhviNarayanan:rvvilogger
Closed

RVVILogger Module to RVVI TEXT format for coverage#1584
SadhviNarayanan wants to merge 112 commits intoopenhwgroup:mainfrom
SadhviNarayanan:rvvilogger

Conversation

@SadhviNarayanan
Copy link
Contributor

This is a very initial, foundational version of the RVVI logger module, which uses the RVVI Trace to extract internal hart signals and write them out to an RVVI TEXT file in the format it requires.
Note: the output has not been validated with the Python script, I just wanted to get some initial feedback on the module to see if it was even progressing in the right direction.

Copy link
Member

@jordancarlin jordancarlin left a comment

Choose a reason for hiding this comment

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

This is looking like a great start! I left some initial comments. Let me know when you’ve had a chance to actually run it and validate it with the Python script and then I’ll take another look.

$fwrite(file, " %5h ", rvvi.pc_rdata[retire_slot][hart][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "%6h ", rvvi.pc_rdata[retire_slot][hart][63:0]);
end else begin
Copy link
Member

Choose a reason for hiding this comment

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

Why is this else case needed?

Copy link
Member

Choose a reason for hiding this comment

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

Global comment. A lot of if blocks seem to have unnecessary else cases.

// Get register name
reg_name = getGPRName(i);

// Format: X <index> <value>
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn’t match what you’re actually printing.

$fwrite(file, "F %2d '%s' %08h ", i, reg_name, rvvi.f_wdata[retire_slot][hart][i][31:0]);
end else if (FLEN == 64) begin
$fwrite(file, "F %2d '%s' %016h ", i, reg_name, rvvi.f_wdata[retire_slot][hart][i][63:0]);
end else if (FLEN == 128) begin
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way to use FLEN in the format specifiers so we don’t need all of these if/else cases? Same applies to XLEN elsewhere.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new SystemVerilog module rvviTextLogger that converts RVVI-TRACE interface signals to RVVI-TEXT format for use with coverage tools and reference models. This is an initial foundational version for early feedback.

Key Changes

  • Adds a fully parameterized RVVI-TRACE to RVVI-TEXT converter module
  • Implements file-based logging of instruction retirement, register changes, CSR updates, and processor state
  • Provides support for multi-hart, superscalar processors with configurable parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 141 to 143
$fwrite(file, " %5h ", rvvi.pc_rdata[retire_slot][hart][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "%6h ", rvvi.pc_rdata[retire_slot][hart][63:0]);
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The PC formatting uses %5h for 32-bit addresses and %6h for 64-bit addresses, but these format specifiers limit the output to 5 and 6 hex digits respectively. A 32-bit address requires 8 hex digits (0x00000000 to 0xFFFFFFFF), and a 64-bit address requires 16 hex digits. The current format will truncate leading digits.

Suggested change
$fwrite(file, " %5h ", rvvi.pc_rdata[retire_slot][hart][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "%6h ", rvvi.pc_rdata[retire_slot][hart][63:0]);
$fwrite(file, " %08h ", rvvi.pc_rdata[retire_slot][hart][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "%016h ", rvvi.pc_rdata[retire_slot][hart][63:0]);

Copilot uses AI. Check for mistakes.
Comment on lines 264 to 265
if (virt_changed && rvvi.mode_virt[retire_slot][hart]) begin
$fwrite(file, "VIRT %0d ", rvvi.mode_virt[retire_slot][hart]);
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The VIRT mode tracking only updates prev_mode_virt when the value is non-zero, but it should be updated whenever virt_changed is true. This means if VIRT mode changes from 1 to 0, the change won't be tracked and subsequent transitions may be missed. The update should occur outside the inner condition check.

Suggested change
if (virt_changed && rvvi.mode_virt[retire_slot][hart]) begin
$fwrite(file, "VIRT %0d ", rvvi.mode_virt[retire_slot][hart]);
if (virt_changed) begin
if (rvvi.mode_virt[retire_slot][hart]) begin
$fwrite(file, "VIRT %0d ", rvvi.mode_virt[retire_slot][hart]);
end

Copilot uses AI. Check for mistakes.
Comment on lines 232 to 243
// Iterate through all 4096 possible CSR addresses
// Note: made CSR address is in HEX format
for (int i = 0; i < 4096; i++) begin
if (rvvi.csr_wb[retire_slot][hart][i]) begin
// Format: C <address_hex> <value_hex>
if (XLEN == 32) begin
$fwrite(file, "C %03h %08h ", i, rvvi.csr[retire_slot][hart][i][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "C %03h %016h ", i, rvvi.csr[retire_slot][hart][i][63:0]);
end else begin
$fwrite(file, "C %03h %h ", i, rvvi.csr[retire_slot][hart][i]);
end
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

Iterating through all 4096 CSR addresses for every retired instruction is inefficient. Most instructions don't write to CSRs, and when they do, they typically only write to one or a few. Consider using a data structure to track only the CSRs that have been written, or if the RVVI interface provides a way to query which CSRs were written, use that instead of checking all 4096 addresses.

Suggested change
// Iterate through all 4096 possible CSR addresses
// Note: made CSR address is in HEX format
for (int i = 0; i < 4096; i++) begin
if (rvvi.csr_wb[retire_slot][hart][i]) begin
// Format: C <address_hex> <value_hex>
if (XLEN == 32) begin
$fwrite(file, "C %03h %08h ", i, rvvi.csr[retire_slot][hart][i][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "C %03h %016h ", i, rvvi.csr[retire_slot][hart][i][63:0]);
end else begin
$fwrite(file, "C %03h %h ", i, rvvi.csr[retire_slot][hart][i]);
end
// Iterate only over written CSR addresses using rvvi.csr_wb_list and rvvi.csr_wb_count
// Note: CSR address is in HEX format
for (int idx = 0; idx < rvvi.csr_wb_count[retire_slot][hart]; idx++) begin
int i = rvvi.csr_wb_list[retire_slot][hart][idx];
// Format: C <address_hex> <value_hex>
if (XLEN == 32) begin
$fwrite(file, "C %03h %08h ", i, rvvi.csr[retire_slot][hart][i][31:0]);
end else if (XLEN == 64) begin
$fwrite(file, "C %03h %016h ", i, rvvi.csr[retire_slot][hart][i][63:0]);
end else begin
$fwrite(file, "C %03h %h ", i, rvvi.csr[retire_slot][hart][i]);

Copilot uses AI. Check for mistakes.
dependabot bot and others added 27 commits January 15, 2026 13:37
Bumps [addins/riscv-arch-test-cvw](https://github.com/riscv-non-isa/riscv-arch-test) from `c08ddfd` to `630dda6`.
- [Release notes](https://github.com/riscv-non-isa/riscv-arch-test/releases)
- [Commits](riscv-non-isa/riscv-arch-test@c08ddfd...630dda6)

---
updated-dependencies:
- dependency-name: addins/riscv-arch-test-cvw
  dependency-version: 630dda65e47280db6af293e0cf575d7c9bea718e
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
…dules/addins/riscv-arch-test-cvw-630dda6

Bump addins/riscv-arch-test-cvw from `c08ddfd` to `630dda6`
got e85 processor running on tests and synthasis running with sky130
Act4 running in lynn directory and tohost changes made for sample processor (SW fixed as well)
Bumps [addins/riscv-arch-test-cvw](https://github.com/riscv-non-isa/riscv-arch-test) from `630dda6` to `8e42a66`.
- [Release notes](https://github.com/riscv-non-isa/riscv-arch-test/releases)
- [Commits](riscv-non-isa/riscv-arch-test@630dda6...8e42a66)

---
updated-dependencies:
- dependency-name: addins/riscv-arch-test-cvw
  dependency-version: 8e42a6640df5d2885942596f23fd203afb9037d4
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
…dules/addins/riscv-arch-test-cvw-8e42a66

Bump addins/riscv-arch-test-cvw from `630dda6` to `8e42a66`
Migrate to uv for Python and update GCC
Update pre-commit hook installation to use uv after Python refactor
Bumps [addins/riscv-arch-test-cvw](https://github.com/riscv-non-isa/riscv-arch-test) from `8e42a66` to `1198f62`.
- [Release notes](https://github.com/riscv-non-isa/riscv-arch-test/releases)
- [Commits](riscv-non-isa/riscv-arch-test@8e42a66...1198f62)

---
updated-dependencies:
- dependency-name: addins/riscv-arch-test-cvw
  dependency-version: 1198f6295e4f93db5ae7024b16835887dd0baf6c
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
…dules/addins/riscv-arch-test-cvw-1198f62

Bump addins/riscv-arch-test-cvw from `8e42a66` to `1198f62`
@jordancarlin
Copy link
Member

jordancarlin commented Jan 24, 2026

@SadhviNarayanan something seems to have gone wrong here. It's showing 100+ files changed.

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.

7 participants