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

Jump to bottom

AUTOINST enhancement (range expressions) #1346

Closed
veripoolbot opened this issue Sep 13, 2018 · 6 comments
Closed

AUTOINST enhancement (range expressions) #1346

veripoolbot opened this issue Sep 13, 2018 · 6 comments
Labels
autos

Comments

@veripoolbot
Copy link
Collaborator

Author Name: Maghawan Punde
Original Redmine Issue: 1346 from https://www.veripool.org

Original Assignee: Maghawan Punde

I want to request an enhancement to Verilog mode AUTOINST.

Here is snippet of ports in module being instantiated.

    output [w2*w1:0]   y0;
    output [w2/w1:0]   y1;
    output [w2>>w1:0]  y2;
    output [w2>>>w1:0] y3;
    output [w2-w1:0]   y4;
    output [w2+w1:0]   y5;
    output [w2<<w1:0]  y6;

Here is how it expands for me.

    output [8:0]         y0;  // From i0_design of design.v
    output [(4)/(2):0]   y1;  // From i0_design of design.v
    output [(4)>>(2):0]  y2;  // From i0_design of design.v
    output [(4)>>>(2):0] y3;  // From i0_design of design.v
    output [2:0]         y4;  // From i0_design of design.v
    output [6:0]         y5;  // From i0_design of design.v
    output [(4)<<(2):0]  y6;  // From i0_design of design.v

Will it be possible to evaluate expressions with "/", "<<", ">>", "<<<" or ">>>"?

=============================
Emacs  : GNU Emacs 26.1 (build 1, x86_64-w64-mingw32)
 of 2018-05-30
Package: verilog-mode v2018-07-18-b1e6a89-vpo
=============================
@veripoolbot
Copy link
Collaborator Author

Original Redmine Comment
Author Name: Maghawan Punde
Original Date: 2018-09-13T14:28:22Z

Attaching patch that worked for me.

@veripoolbot
Copy link
Collaborator Author

Original Redmine Comment
Author Name: Maghawan Punde
Original Date: 2018-09-13T18:18:45Z

Note that verilog code with syntax errors like "output [w2<>w1:0] y2;" would reduce to "[:0]".

The suggested patch (line 10581) may be changed to
"\([0-9]+\)\s \([/]\|[<]\{2,3\}\|[>]\{2,3\}\)\s *\([0-9]+\)"

so that above verilog code reduces to "[4<>2:0] y2".

Either is fine.

@veripoolbot
Copy link
Collaborator Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-13T23:06:14Z

Good work.

Can you please create an updated patch?

;; For precedence do *,/,>>,<< before +,-

  1. This is incorrect, it needs to be *,/ then +,- then <<,<<<,>>,>>>. (See IEEE Operator precedence table.)

  2. Include the repair you mentioned for errors.

  3. Create a new test case in the tests/ directory, and make sure it tries the precedence permutations.

Thanks!

@veripoolbot
Copy link
Collaborator Author

Original Redmine Comment
Author Name: Maghawan Punde
Original Date: 2018-09-15T17:15:50Z

1. Attached patch
            (verilog-mode-improved-range-expressions-3.el OR verilog-mode-diff-3.txt) 
    is working as per expectations.

2. Added test\issue_1346_testcase.v and test_ok\issue_1346_testcase.v  and generated pull request (from maghawan:patch-1 to veripool:master)

3. Please review patch and test case.

@veripoolbot
Copy link
Collaborator Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-18T10:38:11Z

Pushed to git and verilog-mode-2018-09-18-74c0fda-vpo.el

I made one change, to fix your earlier issue I think you moved the <> replacement outside the loop, this isn't good as it won't properly replace multiple shifts. I instead make sure there's no leading <>'s. The results passed your tests, but please give it a try.

@veripoolbot
Copy link
Collaborator Author

Original Redmine Comment
Author Name: Maghawan Punde
Original Date: 2018-09-18T11:22:50Z

Thanks. Your release worked fine for me.

Yes, I moved <> replacement outside the loop but then also added another pass of "(while (not (equal last-pass out)", thinking multiple shifts would be replaced correctly (at least as seen in the test I wrote). I'll try to fail my patch and update the test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
autos
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@veripoolbot