Skip to content

Convert enum methods on type-parameter fields. - #331

Open
davekeeshan wants to merge 1 commit into
zachjs:masterfrom
daxzio:enum-method-type-param
Open

Convert enum methods on type-parameter fields.#331
davekeeshan wants to merge 1 commit into
zachjs:masterfrom
daxzio:enum-method-type-param

Conversation

@davekeeshan

Copy link
Copy Markdown

Summary

Adds a new Convert.EnumMethod pass that lowers SystemVerilog enum methods (next, prev, first, last, num) to synthesizable Verilog (IEEE 1800-2017 §6.19.5).

Also fixes conversion when the enum lives inside a type-parameter binding (e.g. a struct field passed via parameter type), which previously required manual preprocessing or left .next() in the output.

This addresses some of the issues raised in #263

Changes

  • Convert.EnumMethod: new pass (runs after Convert.Enum in the main pipeline). Rewrites enum methods to arithmetic or case/mux expressions; caches enum item values at declaration scope.
  • Convert.Enum: preserve Enum nodes in ParamType declarations and in instance type bindings until ParamType inlines them.
  • Convert.ParamType: treat resolved Enum types as simple so enum-bearing bindings are substituted instead of deferred behind ~-tagged template modules.
  • Tests: enum_method, enum_method_scope, enum_method_paramtype; error test for non-constant next(N) step.
  • CHANGELOG: Unreleased entries for the feature and related ParamType/Enum behavior.

Motivation

Real RTL (e.g. PULP Ara ara_dispatcher.sv) uses ara_req.emul.next() where emul is a field of a type passed as parameter type ara_req_t. Without this change, sv2v either leaves .next() in the Verilog or fails once enum types are stripped before the binding is resolved.

Limitations

  • .name() is not converted.
  • Non-constant next/prev step arguments are rejected (see test/error/enum_method_var_step.sv).
  • Enum methods on a type-parameter field convert only when the enum item list is available after ParamType substitution — usually when the parent module (e.g. ara.sv) is in the same sv2v invocation and binds .ara_req_t(ara_req_t). Converting ara_dispatcher.sv alone without that binding still leaves emul.next().

Add EnumMethod pass and teach ParamType to substitute resolved enum-bearing
type-parameter bindings so ara_req.emul.next() converts without a sed patch.
Includes core tests for paramtype enum methods and scoped enum item values.

Co-authored-by: Cursor <cursoragent@cursor.com>
@zachjs

zachjs commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Thank you for putting together this contribution! I have another implementation of enum method support from some time ago that I never quite wrapped up. My concern was that it seemed to slow things down, but it came with another bug fix or two that seemed worthwhile. IIRC, the design I had gone with avoided the first two limitations you listed. If I put this on a branch, would you be interested in reconciling these two designs?

@davekeeshan

Copy link
Copy Markdown
Author

@zachjs Yes absolutely, I am not wedded to a particular way, in fact i was trying to emulate your style as much as I could. Share your branch an I will do my best to get it working, not sure if it is this PR or a different one, but lets get going, I have some time between projects at the moment so I am trying to tidy up some niggles I found along my way before I get stuck into the next one.

@zachjs

zachjs commented Aug 18, 2026

Copy link
Copy Markdown
Owner

in fact i was trying to emulate your style as much as I could.

I noticed! I appreciate your effort.

Share your branch an I will do my best to get it working, not sure if it is this PR or a different one, but lets get going, I have some time between projects at the moment so I am trying to tidy up some niggles I found along my way before I get stuck into the next one.

Thank you! Here you go: https://github.com/zachjs/sv2v/compare/enum-method

@davekeeshan

Copy link
Copy Markdown
Author

I am looking at this now, the only problem is that the code base itself seems to trigger a number of issues, my goal is to get this to compile successfully in icaurs:

  • Enum array dimensions — After resolving a typedef, Enum base items [N-1:0] was flattened with the base range outside the usage range, so slices used the wrong stride (bank * NrBanks+:NrBanks instead of bank * 4+:4).
  • $fatal in functions — Severity tasks inside functions became $display(..., $time) plus $finish, so Icarus would not treat those functions as constant.
  • Unpacked always_comb sensitivity — Explicit event lists named a whole unpacked array (always @(fp8_m_lzc or …)), which Verilog-2005 / Icarus reject.
  • Orphan idx_width() calls — A package function was injected as cf_math_pkg_idx_width, but some unqualified idx_width(...) calls in the same module were left unrewritten.

I have a fix that addresses all of these

@zachjs

zachjs commented Aug 24, 2026

Copy link
Copy Markdown
Owner

I have a fix that addresses all of these

Awesome! Please send it over. I'm happy to help however I can!

I got an email with a comment comparing enum-method with this PR, but it seems it's now gone. Should I ignore the contents of that deleted comment?

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