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
246 stars 90 forks source link

Wrong auto extremes computation when signal is connected to input and output #1844

Closed boorbajones closed 9 months ago

boorbajones commented 9 months ago

Howdy!

Whilst playing with AUTO for a demo I found a curious error, which is a bit "on the edge" in terms of usage, but still not impossible.

It originated from an example of 2nd level metaprogramming, but it holds also if you reduce the matter to a 1st level (e.g.: using the AUTO_TEMPLATE at first round, and then removing the "wrapping" AUTOINSERTLISP code) case.

module c(/*AUTOARG*/);
  output yy;
  input  ww;

  xx[0] = ww;
  /*AUTOINSERTLISP(insert (format "  yy = xx[%d];\n" atop))*/
  /*AUTOINPUT*/
  /*AUTOOUTPUT*/
  /*AUTOWIRE*/
  /*AUTO_LISP(setq atop 5)*/
  /*d AUTO_TEMPLATE (
   .y(xx[@"(1+ @)"]),
   .x(xx[@]),
   )
   */
  /*AUTOINSERTLISP(dotimes (a atop) (insert (format "d d%d(/*AUTOINST\*\/);\n" a)))*/
endmodule

module d(/*AUTOARG*/);
  input  bit x;
  output bit y;

  assign y=!x;
endmodule : d

If you try it you'll see that xx is wrongly sized. The xx[0] input of d0 is not accounted for.

I tried to debug (verilog-auto-wire) calls, but even though I got to the point where I was seeing the two identified lists for inputs and outputs of instantiated submodules, I did not manage to converge to a resolving patch.

This all happened on version 2023.08.27.008533849

Cheers!

Michael.

wsnyder commented 9 months ago

AUTOWIRE only declares based on outputs of an AUTOINST. [0] is not such. You'll need to declare it manually.

wsnyder commented 9 months ago

P.S. To do that

bit [XX_MSB:0] xx;
/*AUTOINSERTLISP(insert (format "  localparam XX_MSB = %d;\n" atop))*/
boorbajones commented 9 months ago

Inspired by your answer I think I found a patch for verilog-auto-wire that works on my example; it also passes the test suite (I had to hack it a bit as my emacs setup saves file without tabs and trailing spaces (e.g.: I had to change the diff step into a diff -w)).

How can I proceed to submit the change? Is a pull request OK?

wsnyder commented 9 months ago

Great! Yes, a pull request is appreciated, please make sure to add a test (and test_ok as golden) as part of the pull, thanks!