Multiplication gets optimized away

[pepijndevos]

The following code produces zero cells on any commit I've tried, so I think it's not something that broke with any recent changes. Changing the multiplication to an addition or making it synchronous reset makes the problem go away.

I tracked the compilation process, and it gets optimized away here: https://github.com/ZirconiumX/74xx-liberty/blob/ca39c8333b0fb642837659dca925dfc37913877a/synth_74.ys#L20

Removing the opt and the opt below it, it produces some output, but simulating it gives incorrect results. I stared at the incorrect output for a while, but it's hard to make sense of the ball of NAND and XOR gates that is supposedly a multiplier.

module counter (
  input rst,
  input clk,
  output led
    
);

    /* reg */
    reg [7:0] counter = 0;
    
    /* assign */
    assign led = counter[7];
    
    /* always */
    always @ (posedge clk or negedge rst) begin
      if (!rst) begin
        counter <= 1;
      end else begin
        counter <= counter * 8'd3;
      end
    end

endmodule

[Ravenslofty]

So, multiplication by a constant should get lowered to shifts and adds (x * 3 == (x << 1) + x, and since the shift is by a constant it is effectively free, making this some 74283s), but I'm unsure what to do about multiplication of two variables.

[pepijndevos]

The problem here is not that multiplication is optimal or not, the problem is that for this program, the output is nothing, the whole program collapses into nothingness, zero gates. Removing optimizations it produces invalid output. No idea what is going on here.

Other than that, I agree on your point about lowering multiplications to shifts and adds.

[Ravenslofty]

I've been experimenting with equiv_opt, which is a Yosys command designed for proving that the design before and after a pass is equivalent. We might need to go back over the passes with it.

[dmsc]

Hi!

module counter (
  input rst,
  input clk,
  output led
    
);

    /* reg */
    reg [7:0] counter = 0;
    
    /* assign */
    assign led = counter[7];
    
    /* always */
    always @ (posedge clk or negedge rst) begin
      if (!rst) begin
        counter <= 1;
      end else begin
        counter <= counter * 8'd3;
      end
    end

endmodule

IMHO, Yosys is correct, your code is equivalent with:

       assign led = 0;

This is because the counter can only be "0", "1", "8" and "64", so the bit 7 is always 0.

[pepijndevos]

Are you sure? My understanding is that 8'd3 is decimal three in 8 bits, but I'm completely new to Verilog and this has messed me up in the past.

What I do know is that removing the opt pass produces incorrect results, and that changing the program to synchronous reset fixes the problem. So it seems unlikely my program is just equal to zero.

What I think is happening and what I've seen happening before is that one of the cells has a wire messed up. This makes subsequent passes think it is not connected, and hence remove it. This also explains why the result without optimizations is incorrect, there is simply a wire missing somewhere.

What I plan to do when I get to it is reduce the bit width to like 2 or 3 so that it becomes easier to inspect. And I also think @ZirconiumX is looking into formal verification to check what we're doing.

[dmsc]

Hi!

Are you sure? My understanding is that 8'd3 is decimal three in 8 bits, but I'm completely new to Verilog and this has messed me up in the past.

You are right, I miss-read. Sorry.

So, values are: 1, 3, 9, 27, 81, 243, 217, ... with 64 different values in the serias.

What I think is happening and what I've seen happening before is that one of the cells has a wire messed up. This makes subsequent passes think it is not connected, and hence remove it. This also explains why the result without optimizations is incorrect, there is simply a wire missing somewhere.

Yea, could be the case here.