Combinational Loops arise when updating from Clash .99

[jamieChipscan]

Hi all,

I'm a bit new to Clash, but am working on bringing some legacy code from .99 up to the most recent version. The original code has lines like:

nsec2 = mux (count .==. 0) nsec nsec2
That previously compiled to

always @(*) begin if(b_1) result_4 = t_1; else result_4 = result_4; end

However, the same lines now introduce a non-stable combinational loop when synthesized.

assign result_2 = b ? s_0 : result_2;
Is there some general solution to these errors, like explicitly adding an intermediary register? I didn't notice note of this in the migration guide or blog posts.

Thanks!

[christiaanb]

Interesting.

When I ask Intel Quartus to create an RTL diagram for verilog generated from:

module Test where

import Clash.Prelude

nsec2 :: SystemClockResetEnable => Signal System (Unsigned 4)
nsec2 = mux (count .==. 0) nsec nsec2
 where
   nsec  = register @System (0 :: Unsigned 4) (nsec + 1)
   count = register (0 :: Unsigned 8) (count + 1)
{-# ANN nsec2 (defSyn "nsec2") #-}

I get:

With the expected level-sensitive latches generated for the following verilog:

wire [3:0] c$result_rec;
assign c$result_rec = b ? nsec : c$result_rec;

And, when I change that to:

reg [3:0] c$result_rec;
always @(*) begin : result_latch
  if (b) begin
    c$result_rec = nsec;
  end else begin
    c$result_rec = c$result_rec;
  end
end

I get the same level-sensitive latch:

However... regardless of what style Clash generated, it doesn't correspond to what we can simulate in Clash; that is when I try:

clashi> sampleN_lazy 8 nsec2 
[0,0,^CInterrupted.

simulation gets stuck trying to output the 3rd sample, which is when count would be 1 (after the 0 on "power-up", followed by another 0 out of reset); and I have to abort with ctrl+c.

So you're correct that the change wasn't mentioned in the migration guide, because we didn't anticipate code such as:

nsec2 = mux (count .==. 0) nsec nsec2

which basically needs level-sensitive latches, which we cannot/do not simulate in Clash. i.e. We only considered code which we can simulate in Clash.

Question: do you know top of your head whether the verilog simulation semantics are different for:

wire [3:0] c$result_rec;
assign c$result_rec = b ? nsec : c$result_rec;

vs

reg [3:0] c$result_rec;
always @(*) begin : result_latch
  if (b) begin
    c$result_rec = nsec;
  end else begin
    c$result_rec = c$result_rec;
  end
end

[christiaanb]

Alright, I changed the circuit slightly to:

nsec2 :: SystemClockResetEnable => Signal System (Index 99)
nsec2 = mux (count .==. 0) nsec nsec2
 where
   nsec  = register @System (maxBound :: Index 99) (satAdd SatWrap 1 <$> nsec)
   count = register (0 :: Unsigned 4) (count + 1)
{-# ANN nsec2 (defSyn "nsec2") #-}

Then in modelsim I see:

for

wire [6:0] c$result_rec;
assign c$result_rec = b ? nsec : c$result_rec;

But when I change that to:

wire [6:0] c$result_rec;
always @(*) begin
  if (b) begin
    c$result_rec = nsec;
  end else begin
    c$result_rec = c$result_rec;
  end
end

then I see:

Which indeed shows that the semantics are definitely different.

I and the rest of the team needs to have a think on how to move forward on this, since we never really took level-sensitive latching into account. Since we don't have a Clash/Haskell simulation/evaluation to compare the Verilog simulation with, it's somewhat difficult to tell which one is the "correct" translation from Haskell to Verilog: the concurrent assignment with the ternary operator, or the always block with blocking assignment.

[christiaanb]

What's interesting is that if I want to replicate the behavior of the following verilog

always @(*) begin
  if (b) begin
    c$result_rec = nsec;
  end else begin
    c$result_rec = c$result_rec;
  end
end

in VHDL, then I need a much smaller sensitivity list:

process (b)
begin
  if b then
    \c$result_rec\ <= nsec;
  else
    \c$result_rec\ <= \c$result_rec\;
  end if;
end process;

[christiaanb]

Another/better option to make the VHDL and Verilog simultion results the same is to add a very small delay:

process (all)
begin
  if b then
    \c$result_rec\ <= nsec after 1 ps;
  else
    \c$result_rec\ <= \c$result_rec\;
  end if;
end process;

[jamieChipscan]

Wow, thank you for the fast and detailed response! We have been temporarily using small delays to move forward with simulation, but as the code is part of a larger shift-register style processor, we are trying to keep things as synchronous as possible (though of course 1 ps is very small).

I will look into the concurrent assignment with the ternary operator vs. always block with blocking assignment question more. I believe the latter is what is expected, though I could be wrong.

[christiaanb]

I've only done (edge-triggered) d-flip-flop based synchronous circuit design during my career, mostly on FPGAs (+ some "asynchronous" design in terms of multi-clock designs with multi-ported SRAM-based memories/fifo's to cross clock domains). Could you explain to me, or perhaps link to a document/website, how one uses gated (level-sensitive) d-latches in synchronous circuit design? That is, I've always associated gated (level-sensitive) d-latches with clockless (what most people call asynchronous) circuit design, so I'm confused/intrigued about your statement that you want to keep things a synchronous as possible but then still use a gated (level-sensitive) d-latch.

[christiaanb]

Like, I understand you (can) build a d-flip-flop using two back-to-back gated d-latches with inverted polarity on the enable, but then the enable is controlled a clock signal. But in the example you posted, the gated d-latch is controlled by a counter .==. 0, which unless it's a 1-bit counter seems unlikely to have the 50% duty-cycle we normally associate with clocks signals. Again, mostly intrigued.

[jamieChipscan]

I'm curious now too, especially since we were able to add registers (regEns, specifically) for count and nsec2 as you did in your first example, and the issue seems to be solved: $_AND_ 138 $_DFF_P_ 833 $_MUX_ 2490 $_NOT_ 42 $_OR_ 126 $_XOR_ 62 all D flip-flops, no latches Yet when I synthesize import Clash.Prelude nsec2 :: SystemClockResetEnable => Signal System (Unsigned 4) nsec2 = mux (count .==. 0) nsec nsec2 where nsec = register @System (0 :: Unsigned 4) (nsec + 1) count = register (0 :: Unsigned 8) (count + 1) {-# ANN nsec2 (defSyn "nsec2") #-} using Yosys, I get === nsec2 === Number of wires: 36 Number of wire bits: 62 Number of public wires: 7 Number of public wire bits: 23 Number of memories: 0 Number of memory bits: 0 Number of processes: 0 Number of cells: 55 $_ANDNOT_ 6 $_DFF_PP0_ 12 $_MUX_ 14 $_NAND_ 4 $_NOR_ 1 $_OR_ 6 $_XNOR_ 4 $_XOR_ 8 4.24. Executing CHECK pass (checking for obvious problems). checking module nsec2.. Warning: found logic loop in module nsec2: cell $abc$293$auto$blifparse.cc:371:parse_blif$315 ($_MUX_) wire \result [0] Warning: found logic loop in module nsec2: cell $abc$293$auto$blifparse.cc:371:parse_blif$316 ($_MUX_) wire \result [1] Warning: found logic loop in module nsec2: cell $abc$293$auto$blifparse.cc:371:parse_blif$317 ($_MUX_) wire \result [2] Warning: found logic loop in module nsec2: cell $abc$293$auto$blifparse.cc:371:parse_blif$318 ($_MUX_) wire \result [3] found and reported 4 problems. so it seems reluctant to add latches, but also does not resolve the logic loop. I'll try and reduce our actual code into a minimized example this afternoon to see why the issue is not arising. I am not a Verilog person and am using Clash explicitly to get out of writing it, so it is good for me to understand the relevant intricacies :)

…

On Wed, Feb 24, 2021 at 11:53 AM Christiaan Baaij ***@***.***> wrote: Like, I understand you (can) build a d-flip-flop using two back-to-back gated d-latches with inverted polarity on the enable, but then the enable is controlled a clock signal. But in the example you posted, the gated d-latch is controlled by a counter .==. 0, which unless it's a 1-bit counter seems unlikely to have the 50% duty-cycle we normally associate with clocks signals. Again, mostly intrigued. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1677 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AP2CA5FDW6I6WO623J7F5ZDTAUVJNANCNFSM4YCSLZUQ> .

[christiaanb]

Just adding some more data points:

wire [6:0] c$result_rec;
always @(*) begin
  if (b) begin
    c$result_rec = nsec;
  end else begin
    c$result_rec = c$result_rec;
  end
end

and

wire [6:0] c$result_rec;
assign #1 c$result_rec = b ? nsec : c$result_rec;

will produce the same simulation output / waveforms in ModelSim.

[jamieChipscan]

After more experimentation, I'm thinking I may be too much of a novice to understand the nuances of your questions. The changes to our processor code equate to just adding an explicit register to nsec2: *module Test2 whereimport Clash.Preludensec2 :: SystemClockResetEnable => Signal System (Unsigned 4)nsec2 = register (0 :: Unsigned 4) (mux (count .==. 3) nsec nsec2) where nsec = register @System (0 :: Unsigned 4) (nsec + 1) count = register (0 :: Unsigned 8) (count + 1){-# ANN nsec2 (defSyn "nsec2") #-}* which behaves correctly in Clash **Test2> sampleN_lazy 8 nsec2[0,0,0,0,0,3,3,3]* and synthesizes to only DFFs *2.23. Printing statistics.=== nsec2 === Number of wires: 41 Number of wire bits: 70 Number of public wires: 7 Number of public wire bits: 23 Number of memories: 0 Number of memory bits: 0 Number of processes: 0 Number of cells: 63 $_ANDNOT_ 5 $_AND_ 5 $_DFF_PP0_ 16 $_MUX_ 18 $_NAND_ 2 $_NOR_ 1 $_OR_ 4 $_XNOR_ 2 $_XOR_ 102.24. Executing CHECK pass (checking for obvious problems).checking module nsec2..found and reported 0 problems.* The simplified diff is: *< wire [3:0] result_1;* *< reg [3:0] result_0 = 4'd0;* *---* *> reg [3:0] c$result_rec;* *< assign result_1 = b ? nsec : result_0;* *< * *< // register begin* *< always @(posedge \c$$d(%,,%) or posedge \c$$d(%,,%)_0 ) begin : result_0_register* *< if ( \c$$d(%,,%)_0 ) begin* *< result_0 <= 4'd0;* *< end else if (\c$$d(%,,%)_1 ) begin* *< result_0 <= result_1;* *< end* *---* *> always @(*) begin : result_latch* *> if (b) begin* *> c$result_rec = nsec;* *> end else begin* *> c$result_rec = c$result_rec;* *> end* *---* *< assign result = result_0;* *---* *> assign result = c$result_rec;* It's worth noting that I haven't gotten a chance to test our modified design in simulation yet, though I will try to tomorrow. When synthesizing your original modification in Yosys, I also get d latches, so it is not just a difference of synthesizers. 2.23. Printing statistics. === nsec2 === Number of wires: 36 Number of wire bits: 62 Number of public wires: 8 Number of public wire bits: 24 Number of memories: 0 Number of memory bits: 0 Number of processes: 0 Number of cells: 55 $_ANDNOT_ 6 $_AND_ 3 $_DFF_PP0_ 12 $_DLATCH_P_ 4 $_MUX_ 10 $_NAND_ 3 $_NOR_ 1 $_OR_ 4 $_XNOR_ 3 $_XOR_ 9 Hopefully the simple addition of the extra register keeps working for me as I bring up more of our design, as the person who wrote it used the *nsec2 = mux (count .==. 0) nsec nsec2* pattern quite often -- I'll let you know if I encounter any of these cases with odd or unsynthesizable behavior.

…

On Wed, Feb 24, 2021 at 1:29 PM Christiaan Baaij ***@***.***> wrote: Just adding some more data points: wire [6:0] c$result_rec;always @(*) begin if (b) begin c$result_rec = nsec; end else begin c$result_rec = c$result_rec; endend and wire [6:0] c$result_rec;assign #1 c$result_rec = b ? nsec : c$result_rec; will produce the same simulation output / waveforms in ModelSim. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1677 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AP2CA5C6JUVVXKLH74SK2Q3TAVAR5ANCNFSM4YCSLZUQ> .

[christiaanb]

I believe the thing that comes closests to the original intent, but is still edge-triggered, would be:

module Test where

import Clash.Prelude

transparentRegister en d = mux en d (regEn undefined en d)

nsec2 :: SystemClockResetEnable => Signal System (Index 99)
nsec2 = transparentRegister (count .==. 0) nsec
 where
   nsec  = register @System (maxBound :: Index 99) (satAdd SatWrap 1 <$> nsec)
   count = register (0 :: Unsigned 4) (count + 1)
{-# ANN nsec2 (defSyn "nsec2") #-}

In the meantime I've been reading up on multi-phase (or at least two-phase) latch designs and pulsed latch designs and trying to think if/how we can neatly model that in Clash. And no worries on not having an answer to my questions, I wasn't necessarily expecting one, it was more me writing down what was going through my mind in trying to come up with a solution/work-around.