Skip to content

feat(lint): Update and lint cache#323

Merged
jjts merged 5 commits intoIObundle:mainfrom
P-Miranda:lint
Nov 21, 2025
Merged

feat(lint): Update and lint cache#323
jjts merged 5 commits intoIObundle:mainfrom
P-Miranda:lint

Conversation

@P-Miranda
Copy link
Copy Markdown
Contributor

  • update with latest py2hwsw commit (+lint fixes)
  • Verilator Lint cache with IOb and AXI4 backends
  • test with:
make lint-test

@jjts jjts requested review from arturum1 and Copilot and removed request for Copilot November 18, 2025 16:37
Copy link
Copy Markdown
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 updates the iob-cache with the latest py2hwsw commit and adds Verilator lint support for both IOb and AXI4 backends. Key changes include removing unused parameters, fixing AXI signal width issues, refactoring code for better maintainability, and adding comprehensive lint waivers.

  • Updated py2hwsw dependency to newer commit
  • Added Verilator lint infrastructure with waiver files for both IOb and AXI4 backends
  • Fixed AXI signal width issues to use proper bit slicing
  • Refactored replacement policy code to use proper signal indexing
  • Extracted reusable RAM module to separate file for better code organization

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
iob_cache.py Added AXI4-specific clock/reset wire, removed unused BE_ADDR_W parameter, updated connection names for consistency
hardware/modules/iob_cache_memory/iob_cache_memory.py Removed unused BE_ADDR_W and DATA_W parameters
hardware/modules/iob_cache_memory/hardware/src/iob_cache_replacement_policy.v Removed unused genvar k, converted mru_index from wire to reg for proper indexed part-select
hardware/modules/iob_cache_memory/hardware/src/iob_cache_memory.v Added OFFSET_PAD_W localparam, fixed clock usage for RAM, added generate block names, improved signal padding
hardware/modules/iob_cache_memory/hardware/src/iob_cache_gen_sp_ram.v New file extracting byte-width generable SP RAM module for reusability
hardware/modules/iob_cache_front_end/iob_cache_front_end.py Removed unused USE_CTRL_CNT parameter
hardware/modules/iob_cache_back_end_iob/hardware/src/iob_cache_write_channel_iob.v Removed unused FE_ADDR_W parameter
hardware/modules/iob_cache_back_end_iob/hardware/src/iob_cache_read_channel_iob.v Fixed word counter increment to avoid implicit conversion
hardware/modules/iob_cache_back_end_axi/iob_cache_back_end_axi.py Updated port from clk_en_rst_s to clk_rst_s, added params field
hardware/modules/iob_cache_back_end_axi/hardware/src/iob_cache_write_channel_axi.v Fixed AXI size signals to use proper 3-bit width, removed unused FE_ADDR_W parameter
hardware/modules/iob_cache_back_end_axi/hardware/src/iob_cache_read_channel_axi.v Fixed AXI size signals to use proper 3-bit width
hardware/modules/iob_cache_back_end_axi/hardware/src/iob_cache_back_end_axi.v Added empty connections for unused axi_arprot_o and axi_awprot_o
hardware/lint/verilator_waiver.vlt New waiver file for suppressing expected Verilator lint warnings
hardware/lint/lint_build.mk New makefile for lint build configuration
default.nix Updated py2hwsw commit hash and SHA256
Makefile Added lint and lint-test targets

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

Comment thread hardware/lint/verilator_waiver.vlt Outdated
lint_off -rule UNUSEDSIGNAL -file "**/*_cache_front_end.v" -match "Signal is not used: 'ctrl_ack_i'*"

// signals used in some generate cases
// but generate bases are split, so we can't just put then inside one of them
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

Typo in comment: "bases" should be "blocks" and "then" should be "them".

Suggested change
// but generate bases are split, so we can't just put then inside one of them
// but generate blocks are split, so we can't just put them inside one of them

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

spelling fixed in dced327

Comment on lines 15 to +17
localparam TAG_W = FE_ADDR_W - (FE_NBYTES_W + WORD_OFFSET_W + NLINES_W);
localparam NWAYS = 2 ** NWAYS_W;
localparam OFFSET_PAD_W = 32 - WORD_OFFSET_W;
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The hardcoded value 32 in OFFSET_PAD_W calculation assumes a maximum width of 32 bits. However, if WORD_OFFSET_W can be greater than or equal to 32, this will result in a negative or zero width, causing synthesis issues. Consider using a calculated width based on the actual signal width needed, or add a parameter validation to ensure WORD_OFFSET_W < 32.

Suggested change
localparam TAG_W = FE_ADDR_W - (FE_NBYTES_W + WORD_OFFSET_W + NLINES_W);
localparam NWAYS = 2 ** NWAYS_W;
localparam OFFSET_PAD_W = 32 - WORD_OFFSET_W;
// Parameter validation: Ensure WORD_OFFSET_W does not exceed FE_DATA_W
generate
if (WORD_OFFSET_W > FE_DATA_W) begin
initial begin
$error("WORD_OFFSET_W (%0d) must be less than or equal to FE_DATA_W (%0d)", WORD_OFFSET_W, FE_DATA_W);
end
end
endgenerate
localparam TAG_W = FE_ADDR_W - (FE_NBYTES_W + WORD_OFFSET_W + NLINES_W);
localparam NWAYS = 2 ** NWAYS_W;
localparam OFFSET_PAD_W = FE_DATA_W - WORD_OFFSET_W;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

word offset is never 32bit, that would mean GBs of word size

Comment on lines +236 to +238
line_wstrb = {{(32-BE_NBYTES){1'b0}}, {BE_NBYTES{read_req_i}}} << (read_addr_i * BE_NBYTES);
end else begin
line_wstrb = (wstrb_reg_i & {FE_NBYTES{write_access}}) << (offset * FE_NBYTES);
line_wstrb = {{(32-FE_NBYTES){1'b0}}, (wstrb_reg_i & {FE_NBYTES{write_access}})} << (offset * FE_NBYTES);
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The hardcoded value 32 assumes a specific maximum signal width for line_wstrb. This should be calculated based on the actual width of line_wstrb which is (2**WORD_OFFSET_W)*FE_NBYTES. Consider using a localparam for this padding width to match the actual signal width.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in dced327

Comment on lines +11 to +12
// For cycle that generated byte-width (single enable) single-port SRAM
// older synthesis tool may require this approch
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

Typo in comment: "For cycle that generated" should be "For cycle that generates". Also "approch" should be "approach".

Suggested change
// For cycle that generated byte-width (single enable) single-port SRAM
// older synthesis tool may require this approch
// For cycle that generates byte-width (single enable) single-port SRAM
// older synthesis tool may require this approach

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

spelling fixed in dced327

@jjts jjts merged commit 4446c0a into IObundle:main Nov 21, 2025
2 checks passed
@P-Miranda P-Miranda deleted the lint branch December 4, 2025 12:26
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.

4 participants