[Verilog/General HDL] Off-by-one error in ispLEVER fitter? Or PEBKAC? (Solved)

[rs20]

Hello,

I have this blob of Verilog -- don't worry about reading the whole lot, the bold parts are the important part.

  input wire drdy;
  input wire dclk;
  input wire din;
  output reg tx;
  output hob;

  reg [7:0] counter = 0;
  reg [7:0] shiftreg;
  reg [5:0] uartcount;

  always @(negedge dclk)
    begin
      shiftreg <= {shiftreg[6:0],din};
      if (drdy)
        counter <= 0;
      else begin
        if (counter[4:0] == 5'd8)
          uartcount <= 6'd3;
        else if (uartcount == 6'd48)
          begin end
        else if (uartcount[2:0] == 3'd0)
          uartcount <= uartcount + 6'd7;
        else
          uartcount <= uartcount + 6'd1;

        if (uartcount[3])
          tx <= shiftreg[uartcount[5:4]];
        else
          tx <= !uartcount[0];

        counter <= counter + 1;
     end
  end

Now, I'm not trying to win the readable code of the year awards here, but it's my understanding that commands within a begin/end block are executed sort-of-sequentially; so on the negative edge of dclk, the shift register first has din shifted into position zero. And then, sort-of-subsequently, tx is assigned to one of shiftreg[0, 1, 2 or 3], based on the contents of uartcount[5:4]. So, the the value of din should be loaded into the tx register on a single clock edge. However, if I compile/fit this using ispLEVER and inspect the fitter's output, we see that the value of din cannot possibly make it to the tx reg in a single clock cycle; it must be transferred to shiftreg_0.Q on one cycle, and then only arrives to tx.Q on the second cycle. AFAICT, this is specifically not what I asked for in my Verilog code above?

  shiftreg_0_.D = din ; (1 pterm, 1 signal)
  shiftreg_0_.C = !dclk ; (1 pterm, 1 signal)

  shiftreg_1_.D = shiftreg_0_.Q ; (1 pterm, 1 signal)
  shiftreg_1_.C = !dclk ; (1 pterm, 1 signal)

  shiftreg_2_.D = shiftreg_1_.Q ; (1 pterm, 1 signal)
  shiftreg_2_.C = !dclk ; (1 pterm, 1 signal)

  shiftreg_3_.D = shiftreg_2_.Q ; (1 pterm, 1 signal)
  shiftreg_3_.C = !dclk ; (1 pterm, 1 signal)

  tx.D = uartcount_3_.Q & shiftreg_0_.Q & !uartcount_4_.Q & !uartcount_5_.Q
      # uartcount_3_.Q & shiftreg_1_.Q & uartcount_4_.Q & !uartcount_5_.Q
      # uartcount_3_.Q & shiftreg_2_.Q & !uartcount_4_.Q & uartcount_5_.Q
      # uartcount_3_.Q & shiftreg_3_.Q & uartcount_4_.Q & uartcount_5_.Q
      # !uartcount_0_.Q & !uartcount_3_.Q ; (5 pterms, 8 signals)
  tx.C = !dclk ; (1 pterm, 1 signal)
  tx.CE = !drdy ; (1 pterm, 1 signal)

So, is the compiler/fitter broken, or is my understanding incorrect?

[rs20]

Well, I can answer my own question; it's a PEBKAC. When I change from the <= operator to the = operator; I see the behaviour I was expecting.

[Bruce Abbott]

So which line(s) did you change, and what was the resulting fitter output?

[rs20]

Ah, my design has gone through a lot of irrelevant changes since I figured out what was going on -- but I can provide I brief summary.

1. First, I changed all instances of <= with =, something like this:

  input wire drdy;
  input wire dclk;
  input wire din;
  output reg tx;
  output hob;

  reg [7:0] counter = 0;
  reg [7:0] shiftreg;
  reg [5:0] uartcount;

  always @(negedge dclk)
    begin
      shiftreg = {shiftreg[6:0],din};
      if (drdy)
        counter = 0;
      else begin
        if (counter[4:0] == 5'd8)
          uartcount = 6'd3;
        else if (uartcount == 6'd48)
          begin end
        else if (uartcount[2:0] == 3'd0)
          uartcount = uartcount + 6'd7;
        else
          uartcount = uartcount + 6'd1;

        if (uartcount[3])
          tx = shiftreg[uartcount[5:4]];
        else
          tx = !uartcount[0];

        counter = counter + 1;
     end
  end

This basically had the effect that I wanted of having the results of assignments being carried over to "subsequent" uses of those same signals -- however, it turns out this makes the fitter equations very complicated. For example, the value of tx depends not only on the previous value of uartcount, but also counter (since uartcount can be set according to the value of counter in "earlier" code).

2. So, armed with the understanding of what was going on, I decided just to embrace the all-updates-only-happen-at-the-end nature of the <= operator; and re-ordered my code so that all reads of any given signal happen before assignments to those signals. This isn't required, but I think it makes the code more readable since I'm no longer relying on the blocking or non-blocking nature of the assignments (note that this code always has a whole pile of other unrelated changes in it, so don't try to identify a 1-to-1 match in functionality):

  input wire drdy;
  input wire dclk;
  input wire din;

  output reg tx = 1;
  output hob;

  reg [7:0] counter = 8'd0;
  reg [7:0] shiftreg;
  reg [5:0] uartcount = 6'd0;

  always @(negedge dclk)
    begin
      if (drdy)
        counter <= 8'd1;
      else begin
        if (uartcount[3])
          // Datums.
          if (uartcount[5:4] == 2'd1)
            tx <= din;
          else if (uartcount[5:4] == 2'd2)
            tx <= shiftreg[1];
          else
            tx <= shiftreg[3];
        else
          tx <= !uartcount[0];

        if (counter[4:0] == 5'd7)
          uartcount <= 6'd23;
        else if (uartcount == 6'd0)
          ;
        else if (uartcount[3:0] == 4'd0)
          uartcount <= uartcount + 6'd7;
        else
          uartcount <= uartcount + 6'd1;
 
        if (counter != 8'd0)
          counter <= counter + 1;
      end

      shiftreg <= {shiftreg[6:0],din};
    end

The downside is that since the code shows the shift happening at the end; I have to manually "pre-shift" the signal in my own mind (hence the tx <= din; line, and the indices being 1 & 3 instead of 2 & 4). Also, it takes a couple of clock cycles for a drdy change to flow into a uartcount change, but that's totally fine since I was waiting 8 cycles anyway and that's just the compromise you have to make for having simpler fitter equations. I don't have the fitter outputs; but it's just exactly what you'd expect from a block of Verilog that now explicitly references the din line; obviously the din is now present in the fitter output since I explicitly called for it.

[ale500]

Not exactly knowing what you have to implement, it is difficult to know why the code you posted doesn't do it. But nonetheless I can tell you the following:

we see that the value of din cannot possibly make it to the tx reg in a single clock cycle; it must be transferred to shiftreg_0.Q on one cycle, and then only arrives to tx.Q on the second cycle. AFAICT, this is specifically not what I asked for in my Verilog code above?

Yes, you asked for it !

Code: [Select]

always @(negedge dclk)
    begin
        shiftreg <= {shiftreg[6:0],din};
        tx <= shiftreg[uartcount[5:4]];
    end

This happens in parallel:

You are loading a FF (shiftreg[0]) with the value of din
You are loading a FF (tx) with the value of shiftreg[0] // let's just assume that uartcount is currently 0

You see ? You asked for it.

The '<=' operator means that on the event (change of the signals as per the sensitivity list of the always) new values are going to be loaded. All '<=' in an always happen in parallel and not sequentially like in C.

Maybe you want to tell us what is that your code is suppose to do, I find that drawing signals (amplitude vs time) helps to visualize what I need, and then I write some HDL to achieve that. A FSM (what you have there with all those counters that doesn't really seem to make much sense to me) diagram can also help.

Why does shiftreg have 8 bits when you only read 4 of them ? uartcount seems not to always get initialized, that is bad.

I'd recommend you get yourself icarus verilog and gtkwave. They are very valuable in understanding/debugging verilog.

You look at the equations but you don't know what exactly your code does and just modify randomly to try to "force" it the way you want it. I been that road before, many times. Don't waste your time that way, I already wasted mine without learning that much.