search

lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.56k stars 762 forks source link

[rv_dm,rtl] Fix vendored RTL which causes a Verilator lint warning #22545

Open rswarbrick opened 6 months ago

rswarbrick commented 6 months ago

Description

The pulp code that we're vendoring for rv_dm contains some code that triggers Verilator warnings about address truncation. We're currently waiving the warning in pulp_riscv_dbg.vlt.

When reviewing the waiver in #22486, @andreaskurth pointed out that a better approach would be to fix the underlying RTL. This issue is to remind us to do so.

andreaskurth commented 6 months ago

@rswarbrick: This has been closed by mistake instead of https://github.com/lowRISC/opentitan/issues/21716 (which I now closed), right? Reopening

rswarbrick commented 6 months ago

Oh, nuts. You're completely right, sorry!

rswarbrick commented 5 months ago

Looking at the task, I think there are two possible approaches.

  1. Convince the pulp authors to tweak the code that we're vendoring, then vendor from the fix.
  2. Apply a fix-up patch in the vendoring process

I'm reasonably convinced that (2) would be a silly thing to do: we'd be waiving a warning by applying an on-the-fly edit to the RTL. This comes with some risk and I can't see a benefit over a lint waiver. So I'm assuming we're going with option (1). Will assign Andreas (as he has much more knowledge of the pulp project than me) and make time estimates accordingly.

andreaskurth commented 5 months ago

Moving to M5 because Verilator is not critical for tapeout signoff

rswarbrick commented 4 months ago

I realise that this is just bumping things down the road(!) but I don't believe this is critical for the M5 milestone. Marking as worthy of discussion in the triage meeting.