YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.43k stars 876 forks source link

SystemVerilog enum types #248

Closed igorangst closed 4 years ago

igorangst commented 7 years ago

Hi, apparently, enums are not supported (yet?) by yosys, even when setting the -sv flag for read_verilog. This small example fails:

module foo(input clk, input rst);
   typedef enum {S0, S1, S2} state_t;
   state_t state;

   always_ff @(posedge clk) begin
    if (rst) begin
      state <= S0;
    end else begin
      state <= state.next();     
    end
  end
endmodule

There are workarounds (rewrite code using localparams or defines), but a better support for SystemVerilog features would be very nice.

AlexDaniel commented 7 years ago

I have stumbled upon this a few days ago too. +1 for this issue.

OrDavidi commented 7 years ago

+1 from me too :)

Kmanfi commented 6 years ago

+1 from me too.

package dd;
    typedef enum logic [1:0] {
        ZER        = 2'd0,
        ONE        = 2'd1
   } t_t;
endpackage
udif commented 6 years ago

I began working on generic typedef >3 months ago, but stopped due to temporary lack of time. I plan revisiting this and other Systemverilog constructs in 1-2 months, if you are patient. My plan was to do:

  1. Add typedef parsing and add to AST (done, but useless without (2) ).
  2. Use typedef with any existing type (e.g. existing wire/logic/reg and any existing array combinations)
  3. Add support for structs. I assume enums can be easily added after (2) as well.

I easily parsed the typedef keyword and added it into the AST. The hard part was using the typedef. Adding a simple rule for using the new type popped up many shift/reduce conflicts (at least 13 IIRC). I have a strategy to handle those, but it will take a large effort to restructure the Bison parser. If I understand correctly (I'm not a compiler writer), and my memory serves me right (again this was over 3 months ago), each time you have something like:

rule1:
  token1 { /* some action */ } token2 |
  token1 { /* some action */ } token3 ;

You will get a shift/reduce conflict, because the Bison parser must make a parsing decision when it gets to the {} part, and cannot look ahead beyond it. If this was restructured to be:

rule1:
  token1 token2{ /* some action */  } |
  token1 token3 { /* some action */ }  ;

You wouldn't get a shift/reduce conflict.

Again, this is what I remember from my temporary branch that I worked on a few months ago. Back then, this looked doable, but takes some time. If you are interested, you may take a look at: https://github.com/udif/yosys/tree/udif_typedef_struct But its 130 commits behind the Yosys master branch (should be easy to rebase, I don't think there were many changes to the Verilog parser, if at all).

mithro commented 6 years ago

FYI - @origintfj

PeterCrozier commented 6 years ago

@udif: I looked at your repo and it looks like you have not had any time to progress your implementation.

I too have been looking at adding typedef to Yosys. I have just pushed some WIP to this fork: [https://github.com/PeterCrozier/yosys/tree/svtypes]().

It implements anonymous enums e.g.

enum integer {IDLE, XX='x, S1='b01, S2='b10} state, next;

and typedef for user types e.g.

typedef reg reg_t;
typedef reg_t reg2_t;
typedef bit[4:0] regno_t;
typedef integer int_t;
typedef enum reg {NO, YES} boolean;

There is support for these statements in packages in the way Yosys supports them with explicit :: operators, e.g.

p::outcomes_t a;
initial a = p::WRONG;

To make this more useful I added import e.g.

import p::*

but I have some structural issues with the way AstNode::simplify() manages identifiers so I currently only support dropping the qualifier on simple user types like

outcomes_t a

and not enum/localparam. Thus this will fail as the RHS won't be recognised from the import of p.

initial a = WRONG;  // fails
initial b = p::WRONG;   // works

There is no support for methods like .next() or for block scope. You still can't put tasks or functions in packages.

I wrote this for my own benefit. All I can say is that it works with my code but it doesn't break any existing Yosys tests. There are no formal tests for the new grammar as yet so it is not suitable for a PR.

I am putting it out there to see if there is any interest in me continuing this. To fix the problems with import will mean bigger changes to AstNode::simplify() than I am comfortable with right now.

swetland commented 5 years ago

FWIW, typedefs, enums, and structs (in that order) are all systemverilog constructs that would be very nice to have.