Improving the AUTOINST handling of signed ports

[fnJeff]

The original issue I brought up several years ago was that AUTOINST would sometimes include bit slices for ports that were signed and cause tools like DC to give warnings about signed to unsigned conversions.

As you can see below, I already have the verilog-auto-inst-vector set to unsigned. This works for one very particular case, but it would be nice if it worked more generally. I will try and explain with testcases.

First, here is the submodule for debugging.

module debug_signed_ports
   (input  logic signed [3:0] a,
    input  logic signed [3:0] b,
    output logic signed [4:0] c
    );
   assign c = a + b;
endmodule

Here is the one case that seems to work. You can see that a, b, and c ports do not add the bit slices that would cause the warnings.

module top;
   logic signed [3:0] a;
   logic signed [3:0] b;
   logic signed [4:0] c;

   /*AUTOLOGIC*/

   /*debug_signed_ports AUTO_TEMPLATE (
    );*/
   debug_signed_ports debug_signed_ports
      (/*AUTOINST*/
       // Outputs
       .c                               (c),
       // Inputs
       .a                               (a),
       .b                               (b));

endmodule

First case that doesn't work is if you try and use AUTOLOGIC to declare the variable for the output port. Here you can see that AUTOLOGIC correctly declared the signed variable, but AUTOINST stopped recognizing it as a signed port and added the bit slice.

module top;
   logic signed [3:0] a;
   logic signed [3:0] b;
//   logic signed [4:0] c;

   /*AUTOLOGIC*/
   // Beginning of automatic wires (for undeclared instantiated-module outputs)
   logic signed [4:0]   c;                      // From debug_signed_ports of debug_signed_ports.v
   // End of automatics

   /*debug_signed_ports AUTO_TEMPLATE (
    );*/
   debug_signed_ports debug_signed_ports
      (/*AUTOINST*/
       // Outputs
       .c                               (c[4:0]),
       // Inputs
       .a                               (a),
       .b                               (b));

endmodule

Second case that doesn't work is using a template to change the name of the variables connected to the ports and using AUTOLOGIC for the output variable. Now you can see that all of them are no longer recognized as signed and the bit slices are included. The first thing you might suggest is just to remove the []. This is a trivial example, but there are cases I have where patterns in the AUTO_TEMPLATE can be used for signals matching both signed and unsigned ports based on naming conventions. So it would really be nicer if AUTOINST was able to determine a signed input from the submodule port declarations and not depend only on local wires of identical names.

module top;
   logic signed [3:0] a_int;
   logic signed [3:0] b_int;
//   logic signed [4:0] c_int;

   /*AUTOLOGIC*/
   // Beginning of automatic wires (for undeclared instantiated-module outputs)
   logic signed [4:0]   c_int;                  // From debug_signed_ports of debug_signed_ports.v
   // End of automatics

   /*debug_signed_ports AUTO_TEMPLATE (
       .\([abc]\)                      (\1_int[]),
    );*/
   debug_signed_ports debug_signed_ports
      (/*AUTOINST*/
       // Outputs
       .c                               (c_int[4:0]),            // Templated
       // Inputs
       .a                               (a_int[3:0]),            // Templated
       .b                               (b_int[3:0]));            // Templated

endmodule

Third case that doesn't work is extra weird because it highlights a possible name clash between submodule port naming and higher level variable naming. If you happen to have higher level variables with the same name as a submodule port that are signed and the same width, AUTOINST then uses the information of the variable matching the submodule port name to determine that the port is signed even if the variable is not connected to or related in any way to the AUTOINST port. That feels dangerous. You can see in this example that I add a declaration for a and c, but those are not used or connected to the submodule. Yet AUTOINST removes the bit slices from the connected wires.

module top;
   logic signed [3:0] a_int;
   logic signed [3:0] b_int;
//   logic signed [4:0] c_int;

   logic signed [3:0] a;
   logic signed [4:0] c;

   /*AUTOLOGIC*/
   // Beginning of automatic wires (for undeclared instantiated-module outputs)
   logic signed         c_int;                  // From debug_signed_ports of debug_signed_ports.v
   // End of automatics

   /*debug_signed_ports AUTO_TEMPLATE (
       .\([abc]\)                      (\1_int[]),
    );*/
   debug_signed_ports debug_signed_ports
      (/*AUTOINST*/
       // Outputs
       .c                               (c_int),                 // Templated
       // Inputs
       .a                               (a_int),                 // Templated
       .b                               (b_int[3:0]));            // Templated

endmodule

I understand this is a bit complicated to solve. It seems AUTOINST has to know it's output is signed and then check what is connected to (also signed and same full bit width) it after AUTO_TEMPLATE and AUTOLOGIC have done their work if necessary.

Debug info:

Emacs : GNU Emacs 27.1 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 3.14.13)
of 2020-12-17
Package: verilog-mode v2021-10-14-797711e-vpo-GNU

current state:
(setq
verilog-active-low-regexp nil
verilog-after-save-font-hook nil
verilog-align-ifelse nil
verilog-assignment-delay ""
verilog-auto-arg-sort nil
verilog-auto-declare-nettype nil
verilog-auto-delete-trailing-whitespace nil
verilog-auto-endcomments nil
verilog-auto-hook nil
verilog-auto-ignore-concat nil
verilog-auto-indent-on-newline t
verilog-auto-inout-ignore-regexp nil
verilog-auto-input-ignore-regexp nil
verilog-auto-inst-column 40
verilog-auto-inst-dot-name nil
verilog-auto-inst-interfaced-ports nil
verilog-auto-inst-param-value nil
verilog-auto-inst-sort nil
verilog-auto-inst-template-numbers nil
verilog-auto-inst-vector 'unsigned
verilog-auto-lineup 'declarations
verilog-auto-newline nil
verilog-auto-output-ignore-regexp nil
verilog-auto-read-includes nil
verilog-auto-reset-blocking-in-non t
verilog-auto-reset-widths t
verilog-auto-save-policy nil
verilog-auto-sense-defines-constant nil
verilog-auto-sense-include-inputs nil
verilog-auto-star-expand t
verilog-auto-star-save nil
verilog-auto-template-warn-unused nil
verilog-auto-tieoff-declaration "wire"
verilog-auto-tieoff-ignore-regexp nil
verilog-auto-unused-ignore-regexp nil
verilog-auto-wire-type "logic"
verilog-before-auto-hook nil
verilog-before-delete-auto-hook nil
verilog-before-getopt-flags-hook nil
verilog-before-save-font-hook nil
verilog-cache-enabled t
verilog-case-fold t
verilog-case-indent 3
verilog-cexp-indent 3
verilog-compiler "verilog "
verilog-coverage "echo 'No verilog-coverage set, see "M-x describe-variable verilog-coverage"'"
verilog-delete-auto-hook nil
verilog-getopt-flags-hook nil
verilog-highlight-grouping-keywords t
verilog-highlight-includes t
verilog-highlight-modules t
verilog-highlight-translate-off nil
verilog-indent-begin-after-if t
verilog-indent-declaration-macros nil
verilog-indent-level 3
verilog-indent-level-behavioral 3
verilog-indent-level-declaration 3
verilog-indent-level-directive 1
verilog-indent-level-module 3
verilog-indent-lists t
verilog-library-directories '(".")
verilog-library-extensions '(".v" ".va" ".sv")
verilog-library-files nil
verilog-library-flags '("")
verilog-linter "echo 'No verilog-linter set, see "M-x describe-variable verilog-linter"'"
verilog-minimum-comment-distance 10
verilog-mode-hook 'verilog-set-compile-command
verilog-mode-release-emacs t
verilog-mode-version "2021-10-14-797711e-vpo-GNU"
verilog-preprocessor "verilator -E FLAGS FILE"
verilog-simulator "echo 'No verilog-simulator set, see "M-x describe-variable verilog-simulator"'"
verilog-tab-always-indent t
verilog-tab-to-comment nil
verilog-typedef-regexp "^\(t_.+\|sm_[^_]+\)$"
verilog-warn-fatal nil
)

[wsnyder]

Thanks for the good write up. I looked at this further.

I understand this is a bit complicated to solve. It seems AUTOINST has to know it's output is signed and then check what is connected to (also signed and same full bit width) it after AUTO_TEMPLATE and AUTOLOGIC have done their work if necessary.

That's a correct summary. Basically at present it does not analyze what the upper connection is at all, and I'm unlikely to add that capability as it will be complicated and hard to get right.

The "verilog-auto-inst-vector 'unsigned" hack was added under the presumption that the connection was not templated, which accounts for some of the bad behavior you note. Perhaps if there's any template the "verilog-auto-inst-vector 'unsigned" should simply be ignored?