Multi-Dimensional array template magic [][] substitution fails when original port name matches parent port name, and verilog-auto-inst-vector is off

[techdude]

I've noticed the following unexpected behavior when verilog-auto-inst-vector is nil. When using a template to connect to a renamed net for a multi-dimensional port (packed), if the module port name is the same as one of the ports on the parent, and has the same dimensions, then the generated comment excludes the last dimension, and as a result any AUTOWIRE, AUTOINPUT, AUTOOUTPUT generated items will have the wrong dimension.

Example to reproduce:

module parent(/*AUTOARG*/);
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

  /*AUTOWIRE*/

  /* child1 AUTO_TEMPLATE (
    .data_out                 (child1_out[][]),
  ); */
  child1 U1 (
    /*AUTOINST*/
  )

  /* child2 AUTO_TEMPLATE (
    .data_in                 (child1_out[][]),
  ); */
  child2 U2 (
    /*AUTOINST*/
  )

endmodule

module child1(/*AUTOARG*/);
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

endmodule

module child2(/*AUTOARG*/);
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

endmodule

// Local Variables:
// verilog-auto-inst-vector:nil
// End:

When verilog-auto-inst-vector is nil, this generates the incorrect following result:

module parent(/*AUTOARG*/
  // Outputs
  data_out,
  // Inputs
  data_in
  );
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

  /*AUTOWIRE*/
  // Beginning of automatic wires (for undeclared instantiated-module outputs)
  wire [31:0]           child1_out;             // From U1 of child1.v
  // End of automatics

  /* child1 AUTO_TEMPLATE (
    .data_out                 (child1_out[][]),
  ); */
  child1 U1 (
    /*AUTOINST*/
    // Outputs
    .data_out                           (child1_out/*[31:0]*/),  // Templated
    // Inputs
    .data_in                            (data_in/*[31:0]*/));

  /* child2 AUTO_TEMPLATE (
    .data_in                 (child1_out[][]),
  ) */
  child2 U2 (
    /*AUTOINST*/
    // Outputs
    .data_out                           (data_out/*[31:0]*/),
    // Inputs
    .data_in                            (child1_out/*[31:0]*/));         // Templated

endmodule

module child1(/*AUTOARG*/
  // Outputs
  data_out,
  // Inputs
  data_in
  )
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

endmodule

module child2(/*AUTOARG*/
  // Outputs
  data_out,
  // Inputs
  data_in
  );
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

endmodule

// Local Variables:
// verilog-auto-inst-vector:nil
// End:

Notice how each connection hint comment only includes one of the dimensions, and as a result, the autowire for the net between the two child blocks is missing a dimension.

The current workaround is to set verilog-auto-inst-vector to 'unsigned or t, which produces the correct result:

module parent(/*AUTOARG*/
  // Outputs
  data_out,
  // Inputs
  data_in
  );
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

  /*AUTOWIRE*/
  // Beginning of automatic wires (for undeclared instantiated-module outputs)
  wire [31:0] [7:0]     child1_out;             // From U1 of child1.v
  // End of automatics

  /* child1 AUTO_TEMPLATE (
    .data_out                 (child1_out[][]),
  ); */
  child1 U1 (
    /*AUTOINST*/
    // Outputs
    .data_out                           (child1_out/*[31:0][7:0]*/), // Templated
    // Inputs
    .data_in                            (data_in/*[31:0][7:0]*/));

  /* child2 AUTO_TEMPLATE (
    .data_in                 (child1_out[][]),
  ) */
  child2 U2 (
    /*AUTOINST*/
    // Outputs
    .data_out                           (data_out/*[31:0][7:0]*/),
    // Inputs
    .data_in                            (child1_out/*[31:0][7:0]*/)); // Templated

endmodule

module child1(/*AUTOARG*/
  // Outputs
  data_out,
  // Inputs
  data_in
  )
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

endmodule

module child2(/*AUTOARG*/
  // Outputs
  data_out,
  // Inputs
  data_in
  );
  input  [31:0][7:0] data_in;
  output [31:0][7:0] data_out;

endmodule

// Local Variables:
// verilog-auto-inst-vector:'unsigned
// End:

In this case, the comments include both dimensions, and the AUTOWIRE statement produces the correct dimensions for the interconnecting net.

I tracked down the bug a little bit further and it is related to the current logic of verilog-auto-inst-port. vl-bits is set up near the top, however, if verilog-auto-inst-vector is nil, and the port name matched one of the ports in the current module (moddecls), and the last dimension matches (as best as I can tell, based on the return value of verilog-sig-bits), then vl-bits ends up as an empty string, and thus any of the later logic that uses vl-bits, including the hint comment, are missing that dimension range string.

(vl-bits (if (or (eq verilog-auto-inst-vector t)
                          (and (eq verilog-auto-inst-vector `unsigned)
                               (not (verilog-sig-signed port-st)))
              (not (assoc port (verilog-decls-get-signals moddecls)))
              (not (equal (verilog-sig-bits port-st)
                      (verilog-sig-bits
                       (assoc port (verilog-decls-get-signals moddecls))))))
              (or (verilog-sig-bits port-st) "")
            ""))

[wsnyder]

I think the logic you indicate is there to omit suffixes from the 1D packed arrays. Perhaps try making it behave as if verilog-auto-inst-vector for 2D+ arrays?

Can you make this into a pull request, after adding an appropriate test file to test/ (with autos not expanded) and test_ok/ (having what you want as output)?

[techdude]

I don't have a fix for the issue, so nothing to create a pull-request of, however the full test files are above so the issue can be recreated by someone more familiar with the code.

Yes, the logic there is intended to omit suffixes from 1D packed arrays, however the vl-bits value is also used later when outputting the hint for multi-dimensional nets, so if it's not set it produces the incorrect behavior. Possible solution could be just using (verilog-sig-bits port-st) in the hint comment without qualification, though it looks like there is a lot of other substitution logic below that updates vl-bits that I don't understand, and am not sure if it's needed for some other supported syntax.

[techdude]

I added a pull request here: https://github.com/veripool/verilog-mode/pull/1850