Merge pull request #810 from Xilinx/fix/swg_sv_enum

Fix SV enum bug for dynamic SWG

finn-rtllib/swg/swg_common.sv

@@ -1,5 +1,5 @@
 /******************************************************************************
- * Copyright (C) 2022, Advanced Micro Devices, Inc.
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -29,8 +29,10 @@
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *****************************************************************************/
 
+
 // loop controller used for both, "default" and "parallel", implementation styles
-module swg_controller #(
+module swg_controller
+import swg::*; #(
     int unsigned  LOOP_H_ITERATIONS,
     int unsigned  LOOP_W_ITERATIONS,
     int unsigned  LOOP_KH_ITERATIONS,
@@ -50,7 +52,7 @@ module swg_controller #(
     int TAIL_INCR_H,
     int TAIL_INCR_LAST,
 
-    parameter INNERMOST_STATE
+    state_e INNERMOST_STATE
 )(
     input   logic  clk,
     input   logic  rst_n,
@@ -61,14 +63,6 @@ module swg_controller #(
 );
 
     // state and counters
-    typedef enum logic [2:0] {
-        STATE_START,
-        STATE_LOOP_SIMD,
-        STATE_LOOP_KW,
-        STATE_LOOP_KH,
-        STATE_LOOP_W,
-        STATE_LOOP_H
-    }  state_e;
     state_e  State = INNERMOST_STATE;
     state_e  state_next;
 

finn-rtllib/swg/swg_pkg.sv

@@ -0,0 +1,41 @@
+/******************************************************************************
+ * Copyright (C) 2023, Advanced Micro Devices, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ *  1. Redistributions of source code must retain the above copyright notice,
+ *     this list of conditions and the following disclaimer.
+ *
+ *  2. Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ *  3. Neither the name of the copyright holder nor the names of its
+ *     contributors may be used to endorse or promote products derived from
+ *     this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION). HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *****************************************************************************/
+
+package swg;
+	typedef enum logic [2:0] {
+		STATE_START,
+		STATE_LOOP_SIMD,
+		STATE_LOOP_KW,
+		STATE_LOOP_KH,
+		STATE_LOOP_W,
+		STATE_LOOP_H
+	} state_e;
+endpackage : swg

finn-rtllib/swg/swg_template_axilite.v

@@ -1,8 +1,35 @@
+/******************************************************************************
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ *  1. Redistributions of source code must retain the above copyright notice,
+ *     this list of conditions and the following disclaimer.
+ *
+ *  2. Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ *  3. Neither the name of the copyright holder nor the names of its
+ *     contributors may be used to endorse or promote products derived from
+ *     this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION). HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *****************************************************************************/
 
-`timescale 1 ns / 1 ps
-
-module $TOP_MODULE_NAME$_axilite #
-(
+module $TOP_MODULE_NAME$_axilite #(
     // Users to add parameters here
 
     // User parameters ends
@@ -12,8 +39,7 @@ module $TOP_MODULE_NAME$_axilite #
     parameter integer C_S_AXI_DATA_WIDTH	= 32,
     // Width of S_AXI address bus
     parameter integer C_S_AXI_ADDR_WIDTH	= 6
-)
-(
+)(
     // Users to add ports here
     output wire [C_S_AXI_DATA_WIDTH-1:0]	cfg_reg0,
     output wire [C_S_AXI_DATA_WIDTH-1:0]	cfg_reg1,

finn-rtllib/swg/swg_template_default.sv

@@ -98,7 +98,7 @@ module $TOP_MODULE_NAME$_impl #(
         .TAIL_INCR_LAST($TAIL_INCR_LAST$),
         .INCR_BITWIDTH($INCR_BITWIDTH$),
         .IS_DEPTHWISE($IS_DEPTHWISE$),
-        .INNERMOST_STATE($INNERMOST_STATE$)
+        .INNERMOST_STATE(swg::$INNERMOST_STATE$)
     )
     controller_inst (
         .clk(ap_clk),

finn-rtllib/swg/swg_template_default_dynamic.sv

@@ -1,3 +1,34 @@
+/******************************************************************************
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ *  1. Redistributions of source code must retain the above copyright notice,
+ *     this list of conditions and the following disclaimer.
+ *
+ *  2. Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ *  3. Neither the name of the copyright holder nor the names of its
+ *     contributors may be used to endorse or promote products derived from
+ *     this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION). HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *****************************************************************************/
+
 module $TOP_MODULE_NAME$_controller #(
     int unsigned  CNTR_BITWIDTH,
     int unsigned  INCR_BITWIDTH,
@@ -27,6 +58,8 @@ module $TOP_MODULE_NAME$_controller #(
     input logic [INCR_BITWIDTH-1:0] cfg_incr_tail_last
 );
 
+    import  swg::*;
+
     // (dynamic) configuration registers
     logic [CNTR_BITWIDTH-1:0] Cfg_cntr_simd      = $LOOP_SIMD_ITERATIONS$;
     logic [CNTR_BITWIDTH-1:0] Cfg_cntr_kw        = $LOOP_KW_ITERATIONS$;
@@ -62,14 +95,6 @@ module $TOP_MODULE_NAME$_controller #(
     end
 
     // state and counters
-    typedef enum logic [2:0] {
-        STATE_START,
-        STATE_LOOP_SIMD,
-        STATE_LOOP_KW,
-        STATE_LOOP_KH,
-        STATE_LOOP_W,
-        STATE_LOOP_H
-    }  state_e;
     state_e  State = $INNERMOST_STATE$;
     state_e  state_next;
 

finn-rtllib/swg/swg_template_parallel.sv

@@ -123,7 +123,7 @@ module $TOP_MODULE_NAME$_impl #(
         .TAIL_INCR_LAST($TAIL_INCR_LAST$),
         .INCR_BITWIDTH($INCR_BITWIDTH$),
         .IS_DEPTHWISE($IS_DEPTHWISE$),
-        .INNERMOST_STATE($INNERMOST_STATE$)
+        .INNERMOST_STATE(swg::$INNERMOST_STATE$)
     )
     controller_inst (
         .clk(ap_clk),

src/finn/custom_op/fpgadataflow/convolutioninputgenerator_rtl.py

@@ -617,13 +617,13 @@ def prepare_codegen_default(self):
             # skip innermost SIMD loop completely
             if loop_kw_iterations == 1:
                 # skip innermost KW loop completely
-                code_gen_dict["$INNERMOST_STATE$"] = [str(3)]  # STATE_LOOP_KH
+                code_gen_dict["$INNERMOST_STATE$"] = ["STATE_LOOP_KH"]
                 loop_kh_iterations -= 1  # -1 because state is initial state
             else:
-                code_gen_dict["$INNERMOST_STATE$"] = [str(2)]  # STATE_LOOP_KW
+                code_gen_dict["$INNERMOST_STATE$"] = ["STATE_LOOP_KW"]
                 loop_kw_iterations -= 1  # -1 because state is initial state
         else:
-            code_gen_dict["$INNERMOST_STATE$"] = [str(1)]  # STATE_LOOP_SIMD
+            code_gen_dict["$INNERMOST_STATE$"] = ["STATE_LOOP_SIMD"]
             loop_simd_iterations -= 1  # -1 because state is initial state
 
         cntr_bitwidth = math.ceil(
@@ -736,10 +736,10 @@ def prepare_codegen_parallel(self):
         loop_simd_iterations = 1
 
         if loop_w_iterations == 1:
-            code_gen_dict["$INNERMOST_STATE$"] = [str(5)]  # STATE_LOOP_H
+            code_gen_dict["$INNERMOST_STATE$"] = ["STATE_LOOP_H"]
             loop_h_iterations -= 1  # -1 because state is initial state
         else:
-            code_gen_dict["$INNERMOST_STATE$"] = [str(4)]  # STATE_LOOP_W
+            code_gen_dict["$INNERMOST_STATE$"] = ["STATE_LOOP_W"]
             loop_w_iterations -= 1  # -1 because state is initial state
 
         # set head and tail address increment values
@@ -1064,6 +1064,9 @@ def generate_hdl(self):
         shutil.copy2(
             os.environ["FINN_ROOT"] + "/finn-rtllib/swg/swg_common.sv", code_gen_dir
         )
+        shutil.copy2(
+            os.environ["FINN_ROOT"] + "/finn-rtllib/swg/swg_pkg.sv", code_gen_dir
+        )
 
         # set ipgen_path and ip_path so that HLS-Synth transformation
         # and stich_ip transformation do not complain
@@ -1082,6 +1085,7 @@ def prepare_rtlsim(self):
         code_gen_dir = self.get_nodeattr("code_gen_dir_ipgen")
         verilog_paths = [code_gen_dir]
         verilog_files = [
+            "swg_pkg.sv",
             self.get_nodeattr("gen_top_module") + "_wrapper.v",
             self.get_nodeattr("gen_top_module") + "_impl.sv",
             "swg_common.sv",
@@ -1106,6 +1110,7 @@ def code_generation_ipi(self):
         code_gen_dir = self.get_nodeattr("code_gen_dir_ipgen")
 
         sourcefiles = [
+            "swg_pkg.sv",
             self.get_nodeattr("gen_top_module") + "_wrapper.v",
             self.get_nodeattr("gen_top_module") + "_impl.sv",
             "swg_common.sv",

src/finn/util/pyverilator.py

@@ -118,6 +118,8 @@ def file_to_basename(x):
             if not remove_entry:
                 filtered_verilog_files.append(vfile)
             remove_entry = True
+        elif "swg_pkg" in vfile:
+            continue
         else:
             filtered_verilog_files.append(vfile)
 
@@ -315,8 +317,10 @@ def file_to_basename(x):
     xpm_cdc = f"{vivado_path}/data/ip/xpm/xpm_cdc/hdl/xpm_cdc.sv"
     xpm_fifo = f"{vivado_path}/data/ip/xpm/xpm_fifo/hdl/xpm_fifo.sv"
 
+    swg_pkg = os.environ["FINN_ROOT"] + "/finn-rtllib/swg/swg_pkg.sv"
+
     sim = PyVerilator.build(
-        [top_module_file_name, xpm_fifo, xpm_memory, xpm_cdc],
+        [swg_pkg, top_module_file_name, xpm_fifo, xpm_memory, xpm_cdc],
         verilog_path=[vivado_stitch_proj_dir, verilog_header_dir],
         build_dir=build_dir,
         trace_depth=get_rtlsim_trace_depth(),

tests/fpgadataflow/test_fpgadataflow_convinputgenerator_rtl_dynamic.py

@@ -189,6 +189,10 @@ def write_swg_config(sim):
     "ofm": 64,
     "depthwise": True,
     "pad_mode": "SAME_UPPER",
+    # run synthesis for one configuration
+    # this helped expose a bug in enum decls previously
+    # (which config the synth runs on does not matter)
+    "do_synth": True,
 }
 cfg1 = {
     "idims": [(32, 16), (16, 8)],
@@ -198,6 +202,7 @@ def write_swg_config(sim):
     "ofm": 8,
     "depthwise": False,
     "pad_mode": "SAME_UPPER",
+    "do_synth": False,
 }
 cfg2 = {
     "idims": [(64, 128), (2, 4)],
@@ -207,6 +212,7 @@ def write_swg_config(sim):
     "ofm": 64,
     "depthwise": True,
     "pad_mode": "SAME_UPPER",
+    "do_synth": False,
 }
 
 
@@ -215,6 +221,7 @@ def write_swg_config(sim):
 @pytest.mark.vivado
 @pytest.mark.fpgadataflow
 def test_fpgadataflow_conv_dynamic(cfg):
+    do_synth = cfg["do_synth"]
     pad_mode = cfg["pad_mode"]
     depthwise = cfg["depthwise"]
     idims = cfg["idims"]
@@ -292,7 +299,7 @@ def test_fpgadataflow_conv_dynamic(cfg):
     model = model.transform(GiveReadableTensorNames())
     model = model.transform(PrepareIP("xc7z020clg400-1", 5))
     model = model.transform(HLSSynthIP())
-    model = model.transform(CreateStitchedIP("xc7z020clg400-1", 5))
+    model = model.transform(CreateStitchedIP("xc7z020clg400-1", 5, vitis=do_synth))
     model.set_metadata_prop("exec_mode", "rtlsim")
 
     # loop through experiment configurations