[dv] Unpack some randomised structs in DV code

hw/ip/adc_ctrl/dv/env/adc_ctrl_env.core

@@ -11,6 +11,7 @@ filesets:
       - lowrisc:dv:cip_lib
     files:
       - adc_ctrl_env_pkg.sv
+      - adc_ctrl_filter_cfg.sv: {is_include_file: true}
       - adc_ctrl_env_cfg.sv: {is_include_file: true}
       - adc_ctrl_env_var_filter_cfg.sv : {is_include_file: true}
       - adc_ctrl_env_cov.sv: {is_include_file: true}

hw/ip/adc_ctrl/dv/env/adc_ctrl_env_cfg.sv

@@ -36,8 +36,8 @@ class adc_ctrl_env_cfg extends cip_base_env_cfg #(
   rand int pwrup_time;
   rand int wakeup_time;
 
-  // Filter values filter_cfg[channel][filter]
-  rand adc_ctrl_filter_cfg_t filter_cfg[][];
+  // Filter values
+  rand adc_ctrl_filter_cfg filter_cfg[ADC_CTRL_CHANNELS][ADC_CTRL_NUM_FILTERS];
 
   // Debouncing sample counts for normal and low power modes
   rand int np_sample_cnt;
@@ -52,7 +52,12 @@ class adc_ctrl_env_cfg extends cip_base_env_cfg #(
     `uvm_field_int(lp_sample_cnt, UVM_DEFAULT)
   `uvm_object_utils_end
 
-  `uvm_object_new
+  function new(string name="");
+    super.new(name);
+    foreach (filter_cfg[channel, filter]) begin
+      filter_cfg[channel][filter] = new;
+    end
+  endfunction
 
   virtual function void initialize(bit [31:0] csr_base_addr = '1);
     list_of_alerts = adc_ctrl_env_pkg::LIST_OF_ALERTS;
@@ -84,17 +89,6 @@ class adc_ctrl_env_cfg extends cip_base_env_cfg #(
 
   endfunction
 
-  // Constraints
-  // Set the size of the filter_cfg array
-  constraint filter_cfg_size_c {
-    // Size of first dimension
-    filter_cfg.size == ADC_CTRL_CHANNELS;
-
-    // Size of second dimension
-    foreach (filter_cfg[channel]) {filter_cfg[channel].size == ADC_CTRL_NUM_FILTERS;}
-
-  }
-
   // Valid values
   constraint valid_c {
     pwrup_time inside {[0 : 2 ** 4 - 1]};
@@ -117,7 +111,10 @@ class adc_ctrl_env_cfg extends cip_base_env_cfg #(
     // Default filter configuration
     // This is the one assumed for normal use
     foreach (filter_cfg[channel, filter]) {
-      soft filter_cfg[channel][filter] == FILTER_CFG_DEFAULTS[filter];
+      soft filter_cfg[channel][filter].min_v == FILTER_CFG_DEFAULTS[filter].min_v;
+      soft filter_cfg[channel][filter].max_v == FILTER_CFG_DEFAULTS[filter].max_v;
+      soft filter_cfg[channel][filter].match_outside == FILTER_CFG_DEFAULTS[filter].match_outside;
+      soft filter_cfg[channel][filter].enabled == FILTER_CFG_DEFAULTS[filter].enabled;
     }
   }
 
@@ -138,15 +135,12 @@ class adc_ctrl_env_cfg extends cip_base_env_cfg #(
     super.do_print(printer);
 
     // Implement filter_cfg - 2d array of structs
-    printer.print_array_header("filter_cfg", filter_cfg.size);
-    for (int channel = $low(filter_cfg); channel <= $high(filter_cfg); channel++) begin
-      printer.print_array_header($sformatf("filter_cfg[%0d]", channel), filter_cfg[channel].size());
-      for (
-          int filter = $low(filter_cfg[channel]); filter <= $high(filter_cfg[channel]); filter++
-      ) begin
-        printer.print_generic($sformatf("filter_cfg[%0d][%0d]", channel, filter),
-                              "adc_ctrl_filter_cfg_t", $bits(filter_cfg[channel][filter]),
-                              $sformatf("%p", filter_cfg[channel][filter]));
+    printer.print_array_header("filter_cfg", ADC_CTRL_CHANNELS);
+    for (int channel = 0; channel < ADC_CTRL_CHANNELS; channel++) begin
+      printer.print_array_header($sformatf("filter_cfg[%0d]", channel), ADC_CTRL_NUM_FILTERS);
+      for (int filter = 0; filter < ADC_CTRL_NUM_FILTERS; filter++) begin
+        printer.print_object($sformatf("filter_cfg[%0d][%0d]", channel, filter),
+                             filter_cfg[channel][filter]);
       end
       printer.print_array_footer();
     end

hw/ip/adc_ctrl/dv/env/adc_ctrl_env_cov.sv

@@ -20,12 +20,12 @@ class adc_ctrl_filter_cg_wrapper #(
   covergroup adc_ctrl_filter_cg(
       int channel, int filter
   ) with function sample (
-      adc_ctrl_filter_cfg_t cfg, bit is_interrupt = 0, bit is_wakeup = 0, bit clk_gate = 0
+      adc_ctrl_filter_cfg cfg, bit is_interrupt = 0, bit is_wakeup = 0, bit clk_gate = 0
   );
     option.name = $sformatf("adc_ctrl_filter_cg_%0d_%0d", channel, filter);
     option.per_instance = 1;
 
-    cond_cp: coverpoint cfg.cond;
+    match_outside_cp: coverpoint cfg.match_outside;
     min_v_cp: coverpoint cfg.min_v {
       bins minimum = {0};
       bins values[NUMBER_VALUES] = {[1 : (2 ** FILTER_WIDTH - 2)]};
@@ -36,14 +36,14 @@ class adc_ctrl_filter_cg_wrapper #(
       bins values[NUMBER_VALUES] = {[1 : (2 ** FILTER_WIDTH - 2)]};
       bins maximum = {2 ** FILTER_WIDTH - 1};
     }
-    en_cp: coverpoint cfg.en;
+    en_cp: coverpoint cfg.enabled;
     interrupt_cp: coverpoint is_interrupt;
     wakeup_cp: coverpoint is_wakeup;
     clk_gate_cp: coverpoint clk_gate;
-    intr_min_v_cond_xp: cross interrupt_cp, min_v_cp, cond_cp;
-    intr_max_v_cond_xp: cross interrupt_cp, max_v_cp, cond_cp;
-    wakeup_min_v_cond_xp: cross wakeup_cp, min_v_cp, cond_cp;
-    wakeup_max_v_cond_xp: cross wakeup_cp, max_v_cp, cond_cp;
+    intr_min_v_cond_xp: cross interrupt_cp, min_v_cp, match_outside_cp;
+    intr_max_v_cond_xp: cross interrupt_cp, max_v_cp, match_outside_cp;
+    wakeup_min_v_cond_xp: cross wakeup_cp, min_v_cp, match_outside_cp;
+    wakeup_max_v_cond_xp: cross wakeup_cp, max_v_cp, match_outside_cp;
     wakeup_gated_xp : cross wakeup_cp, clk_gate_cp;
   endgroup
 
@@ -96,7 +96,7 @@ class adc_ctrl_env_cov extends cip_base_env_cov #(
   endfunction : build_phase
 
   // Sample filter coverage
-  virtual function void sample_filter_cov(int channel, int filter, adc_ctrl_filter_cfg_t cfg,
+  virtual function void sample_filter_cov(int channel, int filter, adc_ctrl_filter_cfg cfg,
                                           bit is_interrupt = 0, bit is_wakeup = 0,
                                           bit clk_gate = 0);
     adc_ctrl_filter_cg_wrapper_insts[channel][filter].adc_ctrl_filter_cg.sample(cfg, is_interrupt,

hw/ip/adc_ctrl/dv/env/adc_ctrl_env_pkg.sv

@@ -74,39 +74,25 @@ package adc_ctrl_env_pkg;
     AdcCtrlResetModeHw
   } adc_ctrl_reset_mode_e;
 
-  // Filter condition coding
-  typedef enum bit {
-    ADC_CTRL_FILTER_COND_IN  = 0,
-    ADC_CTRL_FILTER_COND_OUT = 1
-  } adc_ctrl_filter_cond_e;
-
-  // Filter configuration
-  typedef struct packed {
-    adc_ctrl_filter_cond_e cond;  // Condition
-    int min_v;  // Minimum value
-    int max_v;  // Maximum value
-    bit en;  // Enable
-  } adc_ctrl_filter_cfg_t;
+  typedef class adc_ctrl_filter_cfg;
 
   // Constants
   // Filter defaults - applies to all channels
-  const
-  adc_ctrl_filter_cfg_t
-  FILTER_CFG_DEFAULTS[] = '{
-      '{min_v: 149, max_v: 279, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 391, max_v: 524, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 712, max_v: 931, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 712, max_v: 847, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 349, max_v: 512, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 405, max_v: 503, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 186, max_v: 279, cond: ADC_CTRL_FILTER_COND_IN, en: 1},
-      '{min_v: 116, max_v: 954, cond: ADC_CTRL_FILTER_COND_OUT, en: 1}
+  const adc_ctrl_filter_cfg FILTER_CFG_DEFAULTS[] = '{
+      adc_ctrl_filter_cfg::make("default0", 149, 279, 1),
+      adc_ctrl_filter_cfg::make("default1", 391, 524, 1),
+      adc_ctrl_filter_cfg::make("default2", 712, 931, 1),
+      adc_ctrl_filter_cfg::make("default3", 712, 847, 1),
+      adc_ctrl_filter_cfg::make("default4", 349, 512, 1),
+      adc_ctrl_filter_cfg::make("default5", 405, 503, 1),
+      adc_ctrl_filter_cfg::make("default6", 186, 279, 1),
+      adc_ctrl_filter_cfg::make("default7", 116, 954, 0)
   };
-  // functions and tasks
 
   // package sources
   `include "adc_ctrl_env_cfg.sv"
   `include "adc_ctrl_env_var_filter_cfg.sv"
+  `include "adc_ctrl_filter_cfg.sv"
   `include "adc_ctrl_env_cov.sv"
   `include "adc_ctrl_virtual_sequencer.sv"
   `include "adc_ctrl_scoreboard.sv"

hw/ip/adc_ctrl/dv/env/adc_ctrl_env_var_filter_cfg.sv

@@ -58,44 +58,61 @@ class adc_ctrl_env_var_filters_cfg extends adc_ctrl_env_cfg;
   }
 
   constraint filters_values_c {
-    solve max_val, min_val, apply_max_v before filter_cfg;
-    foreach (filter_cfg[channel]) {
-      foreach (filter_cfg[channel, filter]) {
-        // Set valid values
-        filter_cfg[channel][filter].min_v inside {[0 : MAX_VALUE]};
-        filter_cfg[channel][filter].max_v inside {[0 : MAX_VALUE]};
-        filter_cfg[channel][filter].max_v >= filter_cfg[channel][filter].min_v;
-
-        // Set first channel to about 1/8 full range so 3/32 to 5/32
-        // then make others within 1/64 range of it so there is some overlap
-        if (channel == 0) {
-          // Channel 0
-          // Make this a soft constraint as it can be broken by minimum and maximum values
-          // If min_v == full range then so must max_v, if max_v == 0 then so must min_v
-          soft (filter_cfg[channel][filter].max_v - filter_cfg[channel][filter].min_v) inside {
-                [THREE_THIRTYSECONDTHS:FIVE_THIRTYSECONDTHS]};
-          // Set maximum/minimum values
-          if (max_val[filter]) {
-            if (apply_max_v[filter]) {
-              filter_cfg[channel][filter].max_v == MAX_VALUE;
-            } else {
-              filter_cfg[channel][filter].min_v == MAX_VALUE;
-            }
+    foreach (filter_cfg[channel, filter]) {
+      // Ordering constraints to make sure that the distribution of max_val, min_val, apply_max_v is
+      // independent of the values for each of the channels / filters.
+      solve max_val, min_val, apply_max_v before filter_cfg[channel][filter].min_v;
+      solve max_val, min_val, apply_max_v before filter_cfg[channel][filter].max_v;
+
+      // Set first channel to about 1/8 full range so 3/32 to 5/32
+      // then make others within 1/64 range of it so there is some overlap
+      if (channel == 0) {
+        // Channel 0
+        // Make this a soft constraint as it can be broken by minimum and maximum values
+        // If min_v == full range then so must max_v, if max_v == 0 then so must min_v
+        soft (filter_cfg[channel][filter].max_v - filter_cfg[channel][filter].min_v) inside {
+              [THREE_THIRTYSECONDTHS:FIVE_THIRTYSECONDTHS]};
+        // Set maximum/minimum values
+        if (max_val[filter]) {
+          if (apply_max_v[filter]) {
+            filter_cfg[channel][filter].max_v == MAX_VALUE;
+          } else {
+            filter_cfg[channel][filter].min_v == MAX_VALUE;
           }
-          if (min_val[filter]) {
-            if (apply_max_v[filter]) {
-              filter_cfg[channel][filter].max_v == 0;
-            } else {
-              filter_cfg[channel][filter].min_v == 0;
-            }
+        }
+        if (min_val[filter]) {
+          if (apply_max_v[filter]) {
+            filter_cfg[channel][filter].max_v == 0;
+          } else {
+            filter_cfg[channel][filter].min_v == 0;
           }
-        } else {
-          // Other channels within 1/64
-          (filter_cfg[channel][filter].min_v - filter_cfg[0][filter].min_v) inside
-            {[-ONE_SIXTYFOURTH : ONE_SIXTYFOURTH]};
-          (filter_cfg[channel][filter].max_v - filter_cfg[0][filter].max_v) inside
-            {[-ONE_SIXTYFOURTH : ONE_SIXTYFOURTH]};
         }
+      } else {
+        // Other channels within ONE_SIXTYFOURTH of the first channel.
+        //
+        // Than taking the difference and claiming it is in a range [-n, n] doesn't work very
+        // cleanly, because the variables are unsigned. To avoid a problem, we can cheat and convert
+        // each claim to two one-sided versions.
+        //
+        // Instead of saying (a - b) inside [-n:n], recast it (in signed integers) as the pair
+        //
+        //    a - b <= n
+        //    b - a <= n
+        //
+        // Then we can get rid of the possible negative numbers by casting it as this pair of
+        // equations over the natural numbers:
+        //
+        //    a <= b + n
+        //    b <= a + n
+        //
+        // That will basically work! But it might overflow (and wrap) if a or b is large.
+        // Practically speaking, this won't matter (because n is an int unsigned), but we can make
+        // it really clear by adding an extra bit to the LHS each time.
+        {1'b0, filter_cfg[channel][filter].min_v} <= filter_cfg[0][filter].min_v + ONE_SIXTYFOURTH;
+        {1'b0, filter_cfg[0][filter].min_v} <= filter_cfg[channel][filter].min_v + ONE_SIXTYFOURTH;
+
+        {1'b0, filter_cfg[channel][filter].max_v} <= filter_cfg[0][filter].max_v + ONE_SIXTYFOURTH;
+        {1'b0, filter_cfg[0][filter].max_v} <= filter_cfg[channel][filter].max_v + ONE_SIXTYFOURTH;
       }
     }
   }
@@ -105,12 +122,10 @@ class adc_ctrl_env_var_filters_cfg extends adc_ctrl_env_cfg;
     $onehot(~mirror_controls);
   }
   constraint filters_control_c {
-    foreach (filter_cfg[channel]) {
-      foreach (filter_cfg[channel, filter]) {
-        if (mirror_controls[filter]) {
-          filter_cfg[channel][filter].cond == filter_cfg[0][filter].cond;
-          filter_cfg[channel][filter].en == filter_cfg[0][filter].en;
-        }
+    foreach (filter_cfg[channel, filter]) {
+      if (mirror_controls[filter]) {
+        filter_cfg[channel][filter].match_outside == filter_cfg[0][filter].match_outside;
+        filter_cfg[channel][filter].enabled == filter_cfg[0][filter].enabled;
       }
     }
   }

hw/ip/adc_ctrl/dv/env/adc_ctrl_filter_cfg.sv

@@ -0,0 +1,67 @@
+// Copyright lowRISC contributors (OpenTitan project).
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// The configuration for a single filter
+
+class adc_ctrl_filter_cfg extends uvm_object;
+  // An interval of values (inclusive).
+  rand bit [ADC_CTRL_DATA_WIDTH-1:0] min_v;
+  rand bit [ADC_CTRL_DATA_WIDTH-1:0] max_v;
+
+  // If match_outside is true, the filter matches a value iff it is not in [min_v, max_v]. If not,
+  // the filter matches a value iff it is *inside* [min_v, max_v].
+  rand bit match_outside;
+
+  // Control whether the filter is enabled
+  rand bit enabled;
+
+  `uvm_object_utils_begin(adc_ctrl_filter_cfg)
+    `uvm_field_int(min_v,        UVM_DEFAULT)
+    `uvm_field_int(max_v,        UVM_DEFAULT)
+    `uvm_field_int(match_outside, UVM_DEFAULT)
+    `uvm_field_int(enabled,      UVM_DEFAULT)
+  `uvm_object_utils_end
+
+  // Ensure that min_v <= max_v
+  extern constraint min_le_max_c;
+
+  extern function new (string name="");
+
+  // A static factory method that returns a filter with the range [min_v, max_v] and the
+  // given match_outside / enabled values.
+  extern static function adc_ctrl_filter_cfg make(string                        name,
+                                                  bit [ADC_CTRL_DATA_WIDTH-1:0] min_v,
+                                                  bit [ADC_CTRL_DATA_WIDTH-1:0] max_v,
+                                                  bit                           match_outside,
+                                                  bit                           enabled = 1'b1);
+endclass
+
+constraint adc_ctrl_filter_cfg::min_le_max_c {
+  min_v <= max_v;
+}
+
+function adc_ctrl_filter_cfg::new (string name="");
+  super.new(name);
+endfunction
+
+function adc_ctrl_filter_cfg
+  adc_ctrl_filter_cfg::make(string                        name,
+                            bit [ADC_CTRL_DATA_WIDTH-1:0] min_v,
+                            bit [ADC_CTRL_DATA_WIDTH-1:0] max_v,
+                            bit                           match_outside,
+                            bit                           enabled = 1'b1);
+  adc_ctrl_filter_cfg ret;
+
+  if (max_v < min_v) begin
+    `uvm_fatal("adc_ctrl_filter_cfg::make",
+               $sformatf("Backwards min_v/max_v range of [%0x, %0x]", min_v, max_v))
+  end
+
+  ret = adc_ctrl_filter_cfg::type_id::create(name);
+  ret.min_v = min_v;
+  ret.max_v = max_v;
+  ret.match_outside = match_outside;
+  ret.enabled = enabled;
+  return ret;
+endfunction