search

clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.4k stars 147 forks source link

Name duplication in generated Verilog for ClockWizard testbench #2598

Closed christiaanb closed 8 months ago

christiaanb commented 8 months ago

On CI we can see the ClockWizard test fails: https://gitlab.com/clash-lang/clash-compiler/-/jobs/5450795505

The underlying reason is that the generated Verilog contains multiple definitions of result:

module topEntity
    ( // Inputs
      input wire  clkInSE // clock
    , input wire  c$clkInDiff // clock
    , input wire  c$clkInDiff_0 // clock (neg phase)
    , input wire  rstIn // reset

      // Outputs
    , output wire [3:0] result_0
    , output wire [3:0] result_1
    );
  // tests/shouldwork/Xilinx/ClockWizard.hs:25:1-9
  wire [1:0] c$ds_app_arg;
  wire [4:0] z;
  wire [3:0] result_4;
  reg [3:0] result_5 = 4'd0;
  // tests/shouldwork/Xilinx/ClockWizard.hs:25:1-9
  wire [1:0] c$ds_app_arg_0;
  // tests/shouldwork/Xilinx/ClockWizard.hs:25:1-9
  wire [1:0] c$ds_case_alt;
  wire [4:0] z_0;
  wire [3:0] result_6;
  reg [3:0] result_7 = 4'd0;
  // tests/shouldwork/Xilinx/ClockWizard.hs:25:1-9
  wire [1:0] ds;
  wire  c$rst;
  wire  eta1;
  reg  c$app_arg = 1'b1;
  reg  c$app_arg = 1'b1;
  wire  result;
  wire  c$rst_0;
  wire  eta1;
  reg  c$app_arg = 1'b1;
  reg  c$app_arg = 1'b1;
  wire  result;
  wire [7:0] result;

Looking at the Core after flattening, the culprit seems to be:

ClockWizard.$s$fClocksSync(,)_$cclocksResetSynchronizer[8214565720323859477] :: Clash.Clocks.Internal.ClocksSyncClocksInst[8214565720323789020]
                                                                                  (Clash.Signal.Internal.Clock[8214565720323788795]
                                                                                     "DomOut",
                                                                                  Clash.Signal.Internal.Reset[8214565720323788823]
                                                                                    "DomOut")
                                                                                  "DomIn"
                                                                            -> Clash.Clocks.Internal.ClocksSyncClocksInst[8214565720323789020]
                                                                                 (Clash.Signal.Internal.Clock[8214565720323788795]
                                                                                    "DomOut",
                                                                                 Clash.Signal.Internal.Reset[8214565720323788823]
                                                                                   "DomOut")
                                                                                 "DomIn"
                                                                            -> Clash.Signal.Internal.Clock[8214565720323788795]
                                                                                 "DomIn"
                                                                            -> (Clash.Signal.Internal.Clock[8214565720323788795]
                                                                                  "DomOut",
                                                                            Clash.Signal.Internal.Reset[8214565720323788823]
                                                                              "DomOut") after flattenCheap:

λ(c$pllOut[0] :: Clash.Signal.Internal.Clock[8214565720323788795]
                   "DomOut") ->
λ(c$pllOut[2] :: Clash.Signal.Internal.Signal[8214565720323788850]
                   "DomIn"
                   GHC.Types.Bool[3674937295934324744]) ->
λ(clkIn[6989586621679082634] :: Clash.Signal.Internal.Clock[8214565720323788795]
                                  "DomIn") ->
let
  result[56064] :: (Clash.Signal.Internal.Clock[8214565720323788795]
                      "DomOut",
               Clash.Signal.Internal.Reset[8214565720323788823]
                 "DomOut")
  = GHC.Tuple.(,)[3963167672086036486]
      @(Clash.Signal.Internal.Clock[8214565720323788795]
          "DomOut")
      @(Clash.Signal.Internal.Reset[8214565720323788823]
          "DomOut")
      c$pllOut[0][LocalId]
      <setName>"resetSynchronizer"
      (letrec
         eta1[6989586621679083069] :: Clash.Signal.Internal.Reset[8214565720323788823]
                                        "DomOut"
         = Clash.Signal.Internal.unsafeToReset @"DomOut"
             (Clash.Normalize.Primitives.removedArg
                @(Clash.Signal.Internal.KnownDomain[8214565720323788821]
                    "DomOut"))
             (GHC.Classes.not
                (Clash.Explicit.Signal.unsafeSynchronizer
                   @"DomIn"
                   @"DomOut"
                   @GHC.Types.Bool[3674937295934324744]
                   (Clash.Normalize.Primitives.removedArg
                      @(Clash.Signal.Internal.KnownDomain[8214565720323788821]
                          "DomIn"))
                   (Clash.Normalize.Primitives.removedArg
                      @(Clash.Signal.Internal.KnownDomain[8214565720323788821]
                          "DomOut"))
                   (Clash.Normalize.Primitives.removedArg
                      @(Clash.Signal.Internal.Clock[8214565720323788795]
                          "DomIn"))
                   (Clash.Normalize.Primitives.removedArg
                      @(Clash.Signal.Internal.Clock[8214565720323788795]
                          "DomOut"))
                   c$pllOut[2][LocalId]))
         c$app_arg[33763] :: Clash.Signal.Internal.Signal[8214565720323788850]
                               "DomOut"
                               GHC.Types.Bool[3674937295934324744]
         = Clash.Signal.Internal.delay# @"DomOut"
             @GHC.Types.Bool[3674937295934324744]
             (Clash.Normalize.Primitives.removedArg
                @(Clash.Signal.Internal.KnownDomain[8214565720323788821]
                    "DomOut"))
             (Clash.Normalize.Primitives.removedArg
                @(Clash.XException.NFDataX[8214565720323788897]
                    GHC.Types.Bool[3674937295934324744]))
             c$pllOut[0][LocalId]
             <prefixName>"enableGen"
             (Clash.Signal.Internal.Enable[8214565720323789001]
                @"DomOut"
                <prefixName>"clockGen1"
                GHC.Types.True[3891110078048108586])
             GHC.Types.True[3891110078048108586]
             c$app_arg[33762][LocalId]
         c$app_arg[33762] :: Clash.Signal.Internal.Signal[8214565720323788850]
                               "DomOut"
                               GHC.Types.Bool[3674937295934324744]
         = Clash.Signal.Internal.delay# @"DomOut"
             @GHC.Types.Bool[3674937295934324744]
             (Clash.Normalize.Primitives.removedArg
                @(Clash.Signal.Internal.KnownDomain[8214565720323788821]
                    "DomOut"))
             (Clash.Normalize.Primitives.removedArg
                @(Clash.XException.NFDataX[8214565720323788897]
                    GHC.Types.Bool[3674937295934324744]))
             c$pllOut[0][LocalId]
             <prefixName>"enableGen"
             (Clash.Signal.Internal.Enable[8214565720323789001]
                @"DomOut"
                <prefixName>"clockGen1"
                GHC.Types.True[3891110078048108586])
             GHC.Types.True[3891110078048108586]
             (Clash.Signal.Internal.unsafeFromReset @"DomOut"
                eta1[6989586621679083069][LocalId])
         result[33764] :: Clash.Signal.Internal.Reset[8214565720323788823]
                            "DomOut"
         = Clash.Signal.Internal.unsafeToReset @"DomOut"
             (Clash.Normalize.Primitives.removedArg
                @(Clash.Signal.Internal.KnownDomain[8214565720323788821]
                    "DomOut"))
             c$app_arg[33763][LocalId]
      in result[33764][LocalId])
in result[56064][LocalId]

Where we have a let-expression as an argument in an application.

christiaanb commented 8 months ago

There are two work-arounds/fixes:

  1. Fix: Change the rewrite strategy in the flattener
  2. Workaround: Change the Verilog primivites for unsafeToReset and unsafeFromReset to be declaration primitives instead of expression primitive, like their VHDL counterparts. This results in Core that doesn't have a let-expression as an argument in an expression.

I have implemented 1, which seems like the better approach.

leonschoorl commented 8 months ago

2407 also created duplicate signal

christiaanb commented 8 months ago

We've released v1.8.0, which includes a fix for this issue.