YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.32k stars 859 forks source link

Don't care optimisation for $shiftx #878

Open eddiehung opened 5 years ago

eddiehung commented 5 years ago

Steps to reproduce the issue

yosys -p "read_verilog -icells shiftx.v; synth_xilinx; stat"

module top(input A, input [1:0] B, output [5:0] Y);
// Containing $shiftx instantiations
no_opt no_opt(A, B, Y[0]);
no_opt2 no_opt2(A, B, Y[1]);
opt opt(A, B, Y[2]);

// Containing inferred $shiftx
no_opt3 no_opt3(A, B, Y[3]);
opt3 opt3(A, B, Y[4]);
endmodule

module no_opt(input A, input [1:0] B, output Y);
$shiftx #(.A_WIDTH(3), .B_WIDTH(2), .Y_WIDTH(1), .A_SIGNED(0), .B_SIGNED(0))
    s (.A({A,A,A}), .B(B), .Y(Y));
endmodule

module no_opt2(input A, input [1:0] B, output Y);
$shiftx #(.A_WIDTH(4), .B_WIDTH(2), .Y_WIDTH(1), .A_SIGNED(0), .B_SIGNED(0))
    s (.A({1'bx,A,A,A}), .B(B), .Y(Y));
endmodule

module no_opt3(input A, input [1:0] B, output Y);
wire [2:0] A3 = {A,A,A};
assign Y = A3[B];
endmodule

module opt(input A, input [1:0] B, output Y);
$shiftx #(.A_WIDTH(4), .B_WIDTH(2), .Y_WIDTH(1), .A_SIGNED(0), .B_SIGNED(0))
    s (.A({A,A,A,A}), .B(B), .Y(Y));
endmodule

module opt3(input A, input [1:0] B, output Y);
wire [3:0] A4 = {A,A,A,A};
assign Y = A4[B];
endmodule

Expected behavior

no_opt* circuits should be optimised away since all its inputs are the same. Difference between those and opt* is that the latter have A_WIDTH == 2**B_WIDTH exactly.

Actual behavior

no_opt* circuits are not optimised away.

eddiehung commented 5 years ago

Discussion with @cliffordwolf indicates that the problem could be that 1'bx in mux cells are not passed to ABC as 1'bx, but instead set to 1'b0...

eddiehung commented 5 years ago

It appears that ABC does not support 'bx so it should be up to Yosys to perform don't care optimisations. Currently, the BLIF writer inside ABC will set all 'bx to 'b0

eddiehung commented 5 years ago

Revisiting this, here are two even more obvious examples where $shiftx sould be optimised:

// Only one value of A is not 1'bx, should be optimised to Y = A[0]
module test1 (A, B, Y);
  input [15:0] A;
  input [3:0] B;
  output Y;
  \$shiftx  #(
    .A_SIGNED(32'd0),
    .A_WIDTH(32'sd16),
    .B_SIGNED(32'd0),
    .B_WIDTH(32'sd4),
    .Y_WIDTH(32'd1)
  ) u1 (
    .A({{15{1'bx}},A[32]}),
    .B(B[3:0]),
    .Y(Y)
  );
endmodule

// All of A is 1'bx, should be optimised away entirely
module test2 (A, B, Y);
  input [15:0] A;
  input [3:0] B;
  output Y;
  \$shiftx  #(
    .A_SIGNED(32'd0),
    .A_WIDTH(32'sd16),
    .B_SIGNED(32'd0),
    .B_WIDTH(32'sd4),
    .Y_WIDTH(32'd1)
  ) u2  (
    .A({16{1'bx}}),
    .B(B[3:0]),
    .Y(Y)
  );
endmodule

// TODO: All but least-significant 2 bits of A are 1'bx, thus should transform into $shiftx with B_WIDTH=1, etc.

./yosys -p "opt -full; stat" shiftx.v on the above gives:

=== test1 ===

   Number of wires:                  3
   Number of wire bits:             21
   Number of public wires:           3
   Number of public wire bits:      21
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     \$shiftx                        1

=== test2 ===

   Number of wires:                  3
   Number of wire bits:             21
   Number of public wires:           3
   Number of public wire bits:      21
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     \$shiftx                        1

@cliffordwolf Where should these optimisations be done? I can optimise them away using techmap rules, but they're not so much a pattern nor a peephole, so perhaps we should be enhancing opt or wreduce?

cliffordwolf commented 5 years ago

Where should these optimisations be done?

Afaict they are all combinatorical single-cell optimizations. As such they should go into opt_expr.