veripool / verilog-mode

Verilog-Mode for Emacs with Indentation, Hightlighting and AUTOs. Master repository for pushing to GNU, verilog.com and veripool.org.
http://veripool.org/verilog-mode
GNU General Public License v3.0
253 stars 90 forks source link

AUTOINST enhancement (range expressions) #1346

Closed veripoolbot closed 5 years ago

veripoolbot commented 6 years ago

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 commented 6 years ago

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


Attaching patch that worked for me.

veripoolbot commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

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.