Yosys right shift error

[WeneneW]

Version

Yosys 0.39+165

On which OS did this happen?

Linux

Reproduction Steps

Consider the following code: y2 has a bit width of 3 bits, and when it is 3'b111, the value of y should be 1. This means that y can take the value 1. However, after synthesis with Yosys, the value of y is fixed at 1'b0.

module top (
    output wire y,
    output wire [2:0] y2,
    output wire [2:0] y3,
    input wire clk,

    input wire wire1

);

    assign y3 = $unsigned(8'b10100000 + wire1);
    assign y2 = y3 - 1'b1;

    assign y = (8'b10011100 >> y2);

endmodule

The synthesized syn_yosys.v is as follows: (Note the assign y = 1'h0;)

/* Generated by Yosys 0.39+165 (git sha1 22c5ab90d, g++  -fPIC -Os) */

(* src = "rtl.v:1.1-20.10" *)
module top(y, y2, y3, clk, wire1);
  (* src = "rtl.v:5.16-5.19" *)
  input clk;
  wire clk;
  (* src = "rtl.v:7.16-7.21" *)
  input wire1;
  wire wire1;
  (* src = "rtl.v:2.17-2.18" *)
  output y;
  wire y;
  (* src = "rtl.v:3.23-3.25" *)
  output [2:0] y2;
  wire [2:0] y2;
  (* src = "rtl.v:4.23-4.25" *)
  output [2:0] y3;
  wire [2:0] y3;
  assign y2[2] = ~wire1;
  assign y = 1'h0; //=======================
  assign y2[1:0] = { y2[2], y2[2] };
  assign y3 = { 2'h0, wire1 };
endmodule

The testbench file is as follows:

`include "../data/cells_cmos.v"
`include "../data/cells_cyclone_v.v"
`include "../data/cells_verific.v"
`include "../data/cells_xilinx_7.v"
`include "../data/cells_yosys.v"
`include "rtl.v"

module testbench;

    // Outputs
    wire y;
    wire [2:0] y2;
    wire [2:0] y3;

    // Inputs
    reg clk;
    reg wire1;

    // Instantiate the Unit Under Test (UUT)
    top uut (
        .y(y),
        .y2(y2),
        .y3(y3),
        .clk(clk),
        .wire1(wire1)
    );

    initial begin
        // Initialize Inputs
        clk = 0;
        wire1 = 0;

        // Monitor outputs
        $monitor("Time: %d, wire1: %b, y: %b, y2: %b, y3: %b", $time, wire1, y, y2, y3);

        // Test case 1: wire1 = 0
        #10 wire1 = 0;
        #10;

        // Test case 2: wire1 = 1
        #10 wire1 = 1;
        #10;

        // End simulation
        $finish;
    end

    always #5 clk = ~clk;  // Generate clock signal

endmodule

The left side shows the simulation results of the original design, while the right side shows the simulation results of the synthesized design. yosys_5_24.zip

Expected Behavior

The right shift result is correct.

Actual Behavior

The right shift result is incorrect.

[KrystalDelusion]

Is it trying to right shift by a negative value and thereby doing a left shift which will always shift in 0?

[WeneneW]

Thank you for your reply. However, after trying to force wire2 to an unsigned conversion, the result after the right shift remains as previously mentioned.

module top (
    output wire y,
    output wire [2:0] y2,
    output wire [2:0] y3,
    input wire clk,

    input wire wire1

);

    assign y3 = $unsigned(8'b10100000 + wire1);

    assign y2 = $unsigned(y3 - 1'b1);

    assign y = (8'b10011100 >> y2);

endmodule

[aiju]

I can't reproduce this on my machine, please provide a yosys synthesis script.

[WeneneW]

I can't reproduce this on my machine, please provide a yosys synthesis script.

These are all the files:

• rtl.v: The original design.
• identity_testbench.v and yosys_testbench.v: Two identical test files.
• syn_yosys.v: The synthesized code by Yosys.

Commands used for synthesis:

yosys
read_verilog rtl.v
synth
write_verilog syn_yosys.v

Other files can be tested using the following commands:

iverilog -o identity_main identity_testbench.v > icarus_stderr_identity.log 2>&1
vvp -n identity_main -lxt2 > vvp_identity.log

iverilog -o yosys_main yosys_testbench.v > icarus_stderr_yosys.log 2>&1
vvp -n yosys_main -lxt2 > vvp_yosys.log

Finally, you can compare the results of vvp_identity.log and vvp_yosys.log to observe the differences.

all.zip

[aiju]

This is a bug in peepopt_shiftadd.pmg

I suspect it's this check that's faulty here:

    // if a value of var is able to wrap the output, the transformation might give wrong results
    // an addition/substraction can at most flip one more bit than the largest operand (the carry bit)
    // as long as the output can show this bit, no wrap should occur (assuming all signed-ness make sense)
    select ( GetSize(port(add, \Y)) > max(GetSize(port(add, \A)), GetSize(port(add, \B))) )

A minimized test case for this bug:

module shift(
    input wire x,
    output wire y
);

    wire [1:0] x0 = x - 1'b1;

    assign y = 4'b1000 >> x0;
endmodule

with the script

read_verilog test.v
dump
eval -set x 0
peepopt
dump
eval -set x 0

The peephole optimiser tries to optimise A>>B-C into {A,{C{1'b0}}} >> B which is incorrect when the subtraction can overflow. The current check for overflow makes sense for addition but not for subtraction, but I'm not sure what the fix would be.

[aiju]

@phsauter Since you wrote this code, maybe you have a better idea what's going on here :)

[phsauter]

Yes thats definitely on me, I need to rethink the checks here:

// checking some value boundaries as well:
// data[...-c +:W1] is fine for +/-var (pad at LSB, all data still accessible)
// data[...+c +:W1] is only fine for +var(add) and var unsigned
// (+c cuts lower C bits, making them inaccessible, a signed var could try to access them)
// either its an add or the variable port is A (it must be positive)
select (add->type.in($add) || varport == \A)

// -> data[var+c +:W1] (with var signed) is illegal
filter !(!offset_negative && varport_signed)

Its possible there are other illegal combinations that aren't covered yet either. Tomorrow I will write out all cases and fix it. Shouldn't be too difficult.

Thanks for spotting this!