[tabular] Handling of tabular alignment

[msfschaffner]

Tabular alignment is currently not handled by the tool. Below are a couple of test cases for this feature.

example a)

should:

  rand bit [ADDR_WIDTH-1:0]     addr;
  rand rw_e                     read_write;
  rand bit [DATA_WIDTH-1:0]     data;
  rand bit [DATA_WIDTH/8-1:0]   be;
  rand bit [3:0]                gnt_delay;
  rand bit [3:0]                req_delay;
  rand bit [5:0]                rvalid_delay;
  rand bit                      error;

is:

  rand bit [ADDR_WIDTH-1:0]     addr;
  rand rw_e read_write;
  rand bit [DATA_WIDTH-1:0]     data;
  rand bit [DATA_WIDTH/8-1:0]   be;
  rand bit [3:0]                gnt_delay;
  rand bit [3:0]                req_delay;
  rand bit [5:0]                rvalid_delay;
  rand bit error;

example b)

should:

    `uvm_field_int       (addr,             UVM_DEFAULT)
    `uvm_field_enum      (rw_e, read_write, UVM_DEFAULT)
    `uvm_field_int       (be,               UVM_DEFAULT)
    `uvm_field_int       (data,             UVM_DEFAULT)
    `uvm_field_int       (gnt_delay,        UVM_DEFAULT)
    `uvm_field_int       (rvalid_delay,     UVM_DEFAULT)
    `uvm_field_int       (error,            UVM_DEFAULT)

is:

    `uvm_field_int(addr, UVM_DEFAULT)
    `uvm_field_enum(rw_e, read_write, UVM_DEFAULT)
    `uvm_field_int(be, UVM_DEFAULT)
    `uvm_field_int(data, UVM_DEFAULT)
    `uvm_field_int(gnt_delay, UVM_DEFAULT)
    `uvm_field_int(rvalid_delay, UVM_DEFAULT)
    `uvm_field_int(error, UVM_DEFAULT)

example c)

should:

      if (!std::randomize(gnt_delay) with {
        gnt_delay dist {
          min_grant_delay                         :/ 1,
          [min_grant_delay+1 : max_grant_delay-1] :/ 1,
          max_grant_delay                         :/ 1
        };
      }) begin
        `uvm_fatal(`gfn, $sformatf("Cannot randomize grant"))
      end

is:

      if (!std::randomize(gnt_delay) with {
        gnt_delay dist {
          min_grant_delay :/ 1,
          [min_grant_delay + 1 : max_grant_delay - 1] :/ 1,
          max_grant_delay :/ 1
        };
      }) begin
        `uvm_fatal(`gfn, $sformatf("Cannot randomize grant"))
      end

example d)

should:

        addr       == item.addr;
        read_write == item.read_write;
        data       == item.data;
        be         == item.be;

is:

        addr == item.addr;
        read_write == item.read_write;
        data == item.data;
        be == item.be;

example e)

should:

module alert_handler_ping_timer (
  input                                     clk_i,
  input                                     rst_ni,
  input                                     entropy_i,          // from TRNG
  input                                     en_i,               // enable ping testing
  input        [alert_pkg::NAlerts-1:0]     alert_en_i,         // determines which alerts to ping
  input        [alert_pkg::PING_CNT_DW-1:0] ping_timeout_cyc_i, // timeout in cycles
  output logic [alert_pkg::NAlerts-1:0]     alert_ping_en_o,    // enable to alert receivers
  output logic [alert_pkg::N_ESC_SEV-1:0]   esc_ping_en_o,      // enable to esc senders
  input        [alert_pkg::NAlerts-1:0]     alert_ping_ok_i,    // response from alert receivers
  input        [alert_pkg::N_ESC_SEV-1:0]   esc_ping_ok_i,      // response from esc senders
  output logic                              alert_ping_fail_o,  // any of the alert receivers failed
  output logic                              esc_ping_fail_o     // any of the esc senders failed
);

is:

module alert_handler_ping_timer(
    input clk_i,
    input rst_ni,
    input entropy_i,  // from TRNG
    input en_i,  // enable ping testing
    input [alert_pkg::NAlerts-1:0]     alert_en_i,  // determines which alerts to ping
    input [alert_pkg::PING_CNT_DW-1:0] ping_timeout_cyc_i,  // timeout in cycles
    output logic [alert_pkg::NAlerts-1:0]     alert_ping_en_o,  // enable to alert receivers
    output logic [alert_pkg::N_ESC_SEV-1:0]   esc_ping_en_o,  // enable to esc senders
    input [alert_pkg::NAlerts-1:0]     alert_ping_ok_i,  // response from alert receivers
    input [alert_pkg::N_ESC_SEV-1:0]   esc_ping_ok_i,  // response from esc senders
    output logic alert_ping_fail_o,  // any of the alert receivers failed
    output logic esc_ping_fail_o  // any of the esc senders failed
);

example f)

is:

  // Data load/store vif connection
  assign data_mem_vif.clock   = clk;
  assign data_mem_vif.reset   = ~rst_n;
  assign data_mem_vif.request = dut.data_req_o;
  assign data_mem_vif.we      = dut.data_we_o;
  assign data_mem_vif.be      = dut.data_be_o;
  assign data_mem_vif.addr    = dut.data_addr_o;
  assign data_mem_vif.wdata   = dut.data_wdata_o;

should:

  // Data load/store vif connection
  assign data_mem_vif.clock = clk;
  assign data_mem_vif.reset = ~rst_n;
  assign data_mem_vif.request = dut.data_req_o;
  assign data_mem_vif.we = dut.data_we_o;
  assign data_mem_vif.be = dut.data_be_o;
  assign data_mem_vif.addr = dut.data_addr_o;
  assign data_mem_vif.wdata = dut.data_wdata_o;

example g)

should:

    alert_handler_accu i_accu (
      .clk_i,
      .rst_ni,
      .clr_i        ( reg2hw_wrap.class_clr[k]          ),
      .class_trig_i ( hw2reg_wrap.class_trig[k]         ),
      .thresh_i     ( reg2hw_wrap.class_accum_thresh[k] ),
      .accu_cnt_o   ( hw2reg_wrap.class_accum_cnt[k]    ),
      .accu_trig_o  ( class_accum_trig[k]               )
    );

is:

    alert_handler_accu
        i_accu(
            .clk_i,
            .rst_ni,
            .clr_i(reg2hw_wrap.class_clr[k]),
            .class_trig_i(hw2reg_wrap.class_trig[k]),
            .thresh_i(reg2hw_wrap.class_accum_thresh[k]),
            .accu_cnt_o(hw2reg_wrap.class_accum_cnt[k]),
            .accu_trig_o(class_accum_trig[k])
        );

example h)

this is an example for tabular alignment for ternaries

should:

  assign accu_d = (clr_i)                      ? '0            : // clear
                  (class_trig_i && !(&accu_q)) ? accu_q + 1'b1 : // saturate counter at maximum
                                                 accu_q;

is:

  assign accu_d = (clr_i) ? '0            :  // clear
  (class_trig_i && !(&accu_q)) ? accu_q + 1'b1 :  // saturate counter at maximum
  accu_q;

[msfschaffner]

b/145156958

[fangism]

Tabular alignment is a very nice-to-have feature, I concur that it makes reading much easier in many contexts. However, many other issues are ahead of this. When the time comes, this will probably be split into separate bugs, one for each specific schema, and a common blocking bug regarding general internal support for tabular alignment.

Some thoughts:

• prescriptive: write a schema per context where applicable
• detective: auto-detect similar patterns between consecutive token partitions
• potential bike-shedding over alignment of parentheses, and left/right flushing

[fangism]

And, does/should alignment persist across //-comments?

[msfschaffner]

hm... I do not think so. I.e. if it is a full line comment as in

assign xx = ...;
assign yy = ...;
// my comment
assign super_long_name = ...;
assign zz              = ...;

I would argue that this is enough separation to warrant a new alignment block...