Xilinx / finn

Dataflow compiler for QNN inference on FPGAs
https://xilinx.github.io/finn
BSD 3-Clause "New" or "Revised" License
708 stars 225 forks source link

Fix SV enum bug for dynamic SWG #810

Closed maltanar closed 1 year ago

maltanar commented 1 year ago

When synthesizing DSWG instances with the latest dev Vivado raises the following error:

ERROR: [Synth 8-9123] an enum variable may only be assigned the same enum typed variable or one of its values [/scratch/finn/build/code_gen_ipgen_ConvolutionInputGenerator_rtl_0_cf9bvsk5/ConvolutionInputGenerator_rtl_0_impl.sv:73]

This is due to a variable in the generated SystemVerilog code

    // 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 = 1; // this value of 1 is not safe to use for the enum, STATE_LOOP_SIMD should be used

The fix is to use the corresponding enum values (e.g. STATE_LOOP_SIMD instead of 1). However, changing this in the codegen breaks the non-dynamic SWG since it uses the same value as a constructor parameter, so fixing that in turn requires moving the definition of the enum to the global scope. To avoid errors due to possible multiple definitions, I've added ifdef guards around it.

Note that the error was previously not detected because the DSWG unit tests do not include any synthesis. This PR adds synthesis to one of the DSWG testcases to detect future occurances of the issue.