f4pga / f4pga-arch-defs

FOSS architecture definitions of FPGA hardware useful for doing PnR device generation.
https://f4pga.org
ISC License
273 stars 113 forks source link

Unresolved $_TBUF_ cells after Yosys synthesis #1668

Open rw1nkler opened 4 years ago

rw1nkler commented 4 years ago

Here is the minimal example that presents the problem with unresolved $_TBUF_ cells, after the Yosys synthesis. This problem can be observed i.e., in EarlGrey design files produced by sv2v. It is worth to note that the assign statement visible bellow does not produce unresolved cells, whereas the pad_wrapper module does.

top.v:

module top (
    input  wire [1:0] sw_en,
    input  wire [1:0] btn_led,
    output wire [1:0] leds
);

    assign leds[0] = sw_en[0] ? btn_led[0] : 1'bz;
    pad_wrapper wrap1 (sw_en[1], btn_led[1], leds[1]);
endmodule

module pad_wrapper (
    input  wire i_oe,
    input  wire i_out,
    output wire o_out
);

    assign o_out = i_oe ? i_out : 1'bz;
endmodule

The result of Yosys synthesis:

rw1nkler commented 4 years ago

FYI @litghost

litghost commented 4 years ago

Okay, so the example you have here can be expressed in the hardware. I wonder if the _TBUF_ pass needs to be done after flattening? Or maybe a fix to iopadmap pass in yosys?

rw1nkler commented 4 years ago

Here is an update on the $_TBUF_ problem.

  1. The same problem does not happen when the mentioned TBUF cases are separated:

    • tbuf_case1.v:

      • sources:

        module top (
            input  wire sw_oe,
            input  wire btn_iout,
            output wire led
        );
        
            assign led = sw_oe ? btn_iout : 1'bz;
        endmodule
      • output: tbuf_case1
    • tbuf_case2.v:

      • sources:

        module top (
            input  wire sw_oe,
            input  wire btn_iout,
            output wire led
        );
        
            pad_wrapper wrap1 (sw_oe, btn_iout, led);
        endmodule
        
        module pad_wrapper (
            input  wire i_oe,
            input  wire i_out,
            output wire o_out
        );
        
            assign o_out = i_oe ? i_out : 1'bz;
        endmodule
        
      • output: tbuf_case2
  2. The problem of $_TBUF_ occurs with a mixed case (tbuf_mixed.v)

    • sources:

      
      module top (
          input  wire [1:0] sw_oe,
          input  wire [1:0] btn_iout,
          output wire [1:0] led
      );
      
          assign led[0] = sw_oe[0] ? btn_iout[0] : 1'bz;
          pad_wrapper wrap1 (sw_oe[1], btn_iout[1], led[1]);
      endmodule
      
      module pad_wrapper (
          input  wire i_oe,
          input  wire i_out,
          output wire o_out
      );
          assign o_out = i_oe ? i_out : 1'bz;
      endmodule
    • output: mixed_case Which seems to be correct, since:

      • 
        module OBUFT_VPR (
           input  I,
           input  T,
           output O
        );
        
           assign O = (T == 1'b0) ? I : 1'bz
        endmodule
        
        module T_INV (
           input  TI,
           output TO
        );
        
           assign TO = ~TI;
        endmodule
  3. After adding additional tribuf -logic pass after synth_xilinx (https://github.com/SymbiFlow/symbiflow-arch-defs/blob/master/xc/xc7/yosys/synth.tcl#L31) the $_TBUF_ disappears but the logic is broken: logic_err

    • The correct solution is the following: logic_fix
rw1nkler commented 4 years ago

I synthesized the examples above with SymbiFlow.

mkurc-ant commented 4 years ago

@rw1nkler So you get the $_TBUF_ in the middle after synth_xilinx? It looks like Yosys didn't understand that it has to move the "hi-z mux" from wrap to top while flattening the design.

I remember that I've read a part of discussion about how Yosys should detect where to insert an IO buffer. Basically it has to be a direct connection to a top-level port. If you do two or more assigns in the way then it is not considered a top level port. The indirect connection forms when the design is flattened.

rw1nkler commented 4 years ago

You might be right @mkurc.

It seems that the only difference between the:

assign led[0] = sw_oe[0] ? btn_iout[0] : 1'bz;

and

pad_wrapper wrap1 (sw_oe[1], btn_iout[1], led[1]);

is this intermediate wrap1.o_out wire

litghost commented 4 years ago

The indirect connection forms when the design is flattened.

To be more specific, the Verilog spec says that module/cell port connections are "buffered", hence the intermediate wire. However from a practical purpose, that buffer doesn't really exist.

ghost commented 4 years ago

I think that this branch is fixing this issue https://github.com/antmicro/yosys/commits/issue-1737-tbuf-fix It consists of two commits:

  1. First one fixes the optimization of cells connected to output ports wider than 2 signals:

    autoidx 2
    attribute \cells_not_processed 1
    module \top
    wire width 2 output 1 \leds
    wire \wrap2.o_out
    
    cell $mux $xx
    parameter \WIDTH 1
    connect \A 1'z
    connect \B 1'1
    connect \S 1'0
    connect \Y \leds [0]
    end
    
    connect \wrap2.o_out \leds [0]
    end

    broken output (should be like input ILANG):

    autoidx 2
    attribute \cells_not_processed 1
    module \top
    wire width 2 output 1 \leds
    wire \wrap2.o_out
    cell $mux $xx
    parameter \WIDTH 1
    connect \A 1'z
    connect \B 1'1
    connect \S 1'0
    connect \Y \wrap2.o_out
    end
    connect \leds [0] \wrap2.o_out
    end
  2. Optimize wires with width more than 1 connected to cell output ports:

    autoidx 2
    attribute \cells_not_processed 1
    module \top
    wire width 2 output 1 \leds
    
    wire $temp
    wire \wrap2.o_out
    
    cell $mux $xx
    parameter \WIDTH 1
    connect \A 1'z
    connect \B 1'1
    connect \S 1'0
    connect \Y $temp
    end
    
    connect \leds [0] $temp
    connect \wrap2.o_out $temp
    end

    output:

    autoidx 2
    attribute \cells_not_processed 1
    module \top
    wire width 2 output 1 \leds
    wire \wrap2.o_out
    cell $mux $xx
    parameter \WIDTH 1
    connect \A 1'z
    connect \B 1'1
    connect \S 1'0
    connect \Y \leds [0]
    end
    connect \leds [0] \wrap2.o_out
    end

This patch is not a perfect solution as it addresses only one case (output ports). Proper solution would fix this assignment mapping: https://github.com/antmicro/yosys/blob/issue-1737-tbuf-fix/passes/opt/opt_clean.cc#L305. But I think it fixes this issue.

GitHub
antmicro/yosys
Yosys Open SYnthesis Suite. Contribute to antmicro/yosys development by creating an account on GitHub.
GitHub
antmicro/yosys
Yosys Open SYnthesis Suite. Contribute to antmicro/yosys development by creating an account on GitHub.