Skip to content
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.

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

[rom_ctrl, dv] Excluded line coverage for prim_generic** blocks #25706

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KinzaQamar
Copy link
Contributor

No description provided.

@KinzaQamar KinzaQamar requested a review from a team as a code owner December 19, 2024 13:49
@KinzaQamar KinzaQamar requested review from rswarbrick and removed request for a team December 19, 2024 13:49
The signals are tied to parameters inside the instantiations
landing as line coverage hole inside the reports.

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
@@ -39,6 +39,15 @@ Block 1 "3318159292" "data_o = 39'(data_i);"
INSTANCE: tb.dut.u_tl_adapter_rom.u_tlul_data_integ_enc_instr.u_data_gen
Block 1 "3318159292" "data_o = 39'(data_i);"

INSTANCE: tb.dut.gen_rom_scramble_enabled.u_rom.u_rom.u_prim_rom.gen_generic.u_impl_generic
ANNOTATION: "cfg_i is tied to 0 in tb"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be rom_cfg_i (which is the name it has as a dut port). I'd probably also:

  • Say something like "We expect this port to be wired to a constant."
  • Add an assertion in rom_ctrl.sv that says this is constant. Maybe something like ASSERT(CfgStable_A, $stable(rom_cfg_i)) ?

Block 1 "118728425" "assign unused_cfg = (^cfg_i);"

INSTANCE: tb.dut.gen_rom_scramble_enabled.u_rom.u_seed_anchor.u_secure_anchor_buf.gen_generic.u_impl_generic
ANNOTATION: "in_i and out_o are tied to parameters"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs expanding a little to make it "less local". I'd probably suggest something like: "The u_seed_anchor module is a buffer whose inputs are constant because they are tied to parameters. As a result, this continuous assignment line cannot see execution."

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.

2 participants