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

Auto-assignment via pattern matching #892

Open
veripoolbot opened this issue Feb 27, 2015 · 4 comments
Open

Auto-assignment via pattern matching #892

veripoolbot opened this issue Feb 27, 2015 · 4 comments

Comments

@veripoolbot
Copy link
Collaborator


Author Name: Greg Hilton
Original Redmine Issue: 892 from https://www.veripool.org


I'm a fan of the two always block style of coding; one combinational block & one simple synchronous block. The annoyance is there are a lot of repetitive simple assignments: {rhs_prefix}{signal}{rhs_suffix} (<=|=) {lhs_prefix}{signal}{lhs_suffix}; . I'm hoping this can be auto-generated by merging the consents from AUTO_TEMPLATE/AUTO_LISP/AUTOINSERTLISP and AUTORESET. Maybe call it AUTOASSIGN.

Below is an example of want I would like it to generate to. AUTOASSIGN within the always_comb should be easy enough as it is like AUTORESET with the RHS as an expression instead of a value. The always_ff cans might be a little trick because not all the LHS and RLS are listed before auto generating. Maybe expression matching one scope level or pass a label (ex comb_logic) to cross reference? Hopefully the feature request is doable and worthwhile.

  output logic [9:0] foobar,
  input logic start, stop, pause,
  input logic fix, // ECO#2
  input logic clk, rst_n);

logic [9:0]               foobar_next;
logic [9:0] foobar_eco,   foobar_eco_next; // ECO#2
logic [4:0] foo,          foo_next;
logic [4:0] bar,          bar_next;
logic [5:0] down_counter, down_counter_next;

typedef enum logic [1:0] {IDLE,START,PAUSE} state_et;
state_et state, state_next;


always_comb begin : comb_logic
  // default values
  down_counter_next = down_counter - 1;
  /*AUTOASSIGN(\(.*\)_next = \1;)*/
  // Beginning of autoassign for unassigned signals
  bar_next = bar;
  foo_next = foo;
  foobar_next = foobar;
  foobar_eco_next = foobar_eco;
  state_next = state;
  // End of automatics
  
  // update values
  case(state)
     IDLE :
       begin
         down_counter_next = '1;
         foobar_next = '0;
         foo_next = '0;
         bar_next = '0;
         if (start && !stop) state_next = START;
         if (foobar_next == '0) foobar_eco_next = foobar;
         if (state_next == START) foobar_eco_next = foobar_next;
       end
     START :
       begin
         if (down_counter==0) begin
           down_counter_next = 0;
           foo_next = foo + 1;
         end
         if (pause || down_counter==0) begin
          state_next = PAUSE;
         end
       end
     PAUSE :
       begin
         down_counter_next = down_counter;
         if (stop) begin
           state_next = IDLE;
           foobar_next = {foo,bar};
         end
         else if (!pause && down_counter>0) begin
           bar_next = bar + 1;
           state_next = START;
         end
       end
  endcase
end
always_ff @(posedge clk, negedge rst_n) begin
  if (!rst_n) begin
     down_counter <= '1;
     state <= IDLE;
     /*AUTORESET*/
     // Beginning of autoreset for uninitialized flops
     bar <= 5'b0;
     foo <= 5'b0;
     foobar <= 10'b0;
     foobar_eco <= 10'b0;
     // End of automatics
  end
  else begin
     foobar <= fix ? foobar_eco_next : forbar_next; // ECO#2
     /*AUTOASSIGN(\(.*\) <= \1_next;)*/
     // Beginning of autoassign for unassigned signals
     bar <= bar_next;
     down_counter <= down_counter_next;
     foo <= foo_next;
     foobar_eco <= foobar_eco_next;
     state <= state_next;
     // End of automatics
  end
end
endmodule
@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-28T13:17:27Z


So in your first one, this is matching .*_next across all signals that are on the LHS in that block (ala AUTORESET)?

/AUTOASSIGN((.)_next = \1;)*/

I would suggest this be MATCH, INSERT-TEXT, optional ANTI-MATCH e.g.

/AUTOASSIGN("(.)_next","\1_next = \1", "not_foo_next")*/

Also does any sort of text like an existing assignment immediately above suppress inserting an AUTOASSIGN? I would assume so.

Maybe this can even be an extension to AUTORESET.

For the second one

 /*AUTOASSIGN(\(.*\) <= \1_next;)*/

You suggest several possible rules, none of which are presently parsed. How about matching all existing regs? Also we need a different command. Here's the use of the NOT-REGEXP.

 /*AUTOASSIGNREGS("\(.*\)", "\1 <= \1_next;", "_next")*/

If this makes sense can you go through several modules and see if these are sufficient to replace a significant amount of your code?

@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Greg Hilton
Original Date: 2015-03-02T19:38:24Z


I like the ANTI-MATCH concept. Yes, I was think it could be an extension of AUTORESET too. I'm assuming the third argument is optional, infers word boundaries (\b) and support regexp. I'd assume the capabilities for AUTOASSIGN functionality do already exist, but my lisp skills are not enough attempt this myself.

 /*AUTOASSIGN("\(.*\)_next","\1_next = \1;")*/

or

 /*AUTOASSIGN("\(.*\)_ns,"\1_ns = \1_cs;", ".\+_next_ns")*/

For the second one, does what you are suggesting match all existing reg/logic in the file or current module? One module for each file is a common practice, however I have seen cases where designers put a sub-module in the same file. If it is current file it would add enforcement to the one module per module guideline, however it might be some resistance in adoptions. Scoping for current module is preferred over current file, but I can accept current file. I think the current parsing capabilities can get current module. For an always block that uses both AUTOASSIGN/AUTORESET and AUTOASSIGNREGS, AUTOASSIGNREGS will need to execute first and maybe do a re-parse, otherwise AUTOASSIGN/AUTORESET will not know what variables are needed to generate.

As with my AUTOASSIGN assumptions, are the same true for AUTOASSIGNREGS?

 /*AUTOASSIGNREGS("\(\(.*\)_next\)", "\2 <= \1;")*/

or

 /*AUTOASSIGNREGS("\(\(.*\)_next\)", "\2 <= \1;", "not_.\+_next;")*/

What about the exception cases? From my code example this is the "down_counter_next" and "ECO#2". If AUTOASSIGN is like AUTORESET, then will be smart enough to not generate the line "down_counter_next = down_counter;" because I already declared "down_counter_next = down_counter - 1;"? Will AUTOASSIGNREGS have the same capability? Or do I need to put the LHS variable name in the anti-match list?

@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-03-13T21:42:28Z


I was assuming AUTOASSIGNREGS would match all reg/logic in the current module which are not already driven by another module. Verilog-mode is already multi-module-in-file aware (though I personally dislike this style).

AUTOASSIGN would only match the assignments in that always block (ala AUTORESET).

What about the exception cases? From my code example this is the "down_counter_next" and "ECO#2". If AUTOASSIGN is like AUTORESET, then will be smart enough to not generate the line "down_counter_next = down_counter;" because I already declared "down_counter_next = down_counter - 1;"?

Yes.

Will AUTOASSIGNREGS have the same capability? Or do I need to put the LHS variable name in the anti-match list?

I think it can also avoid assigning them.

Can you create as complicated of test cases as possible showing how these should work? Please make sure they include sub-modules with // Outputs that should NOT get included in the new AUTOS.

@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Greg Hilton
Original Date: 2015-04-25T22:51:38Z


Sorry it took so long. Attached is a sample and its expected results based on your feedback.

I tried to put reasonable not-regexp that RTL designers might use, like '@^pre_@' or '@_next_next$@'. I added a sub-module as requested. (I'm not a fan of the style either).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant