WIP: Adding a pass which splits inout wires (+ports).

[mithro]

This pull request is not ready to merge, but I'm stuck and need a review to figure out what I'm doing wrong.

The issues I've run into are;

• Hitting the refcount_wires_ assert in RTLIL::Module::add(RTLIL::Wire *wire). The value is 1 (rather than zero) but I don't understand why.
• The output ports are showing up as input ports on the subcells.
• When do I need to run module->fixup_ports();?

My yosys script is; read_verilog example.v; synth_ice40 -nocarry; ice40_opt -unlut; abc -lut 4; opt_clean; splitinout; show.

The current test Verilog I'm using is as follows;

/* Binary counter displayed on LEDs (the 4 green ones on the right).
 * Changes value about once a second.
 */

`ifndef CLK_MHZ
`define CLK_MHZ 24
`endif

(* blackbox *)
module other (
	input I,
	output O
);
endmodule

(* blackbox *)
module other_io (
	inout IO,
);
endmodule

module top (
	input  clk,

	input  LED2,
	output LED3,

	inout  LED4,
	inout  LED5,
	inout  LED6,
	inout  LED7
);

	wire flash_io0_di;
	wire flash_io1_di;
	wire flash_io2_di;
	wire flash_io3_di;
	SB_IO #(
		.PIN_TYPE(6'b 1010_01),
		.PULLUP(1'b 0)
	) io_buf [3:0] (
		.PACKAGE_PIN({LED2, LED3, LED4, LED5}),
		.OUTPUT_ENABLE({outcnt[0], outcnt[1], outcnt[2], outcnt[3]}),
		.D_OUT_0({outcnt[2], outcnt[3], outcnt[4], outcnt[5]}),
		.D_IN_0({flash_io3_di, flash_io2_di, flash_io1_di, flash_io0_di})
	);

	other o (.I(LED6), .O(LED7));
	other_io o1 (.IO(LED6));
	other_io o2 (.IO(LED7));

	localparam BITS = 4;
        localparam LOG2DELAY = $clog2($rtoi(`CLK_MHZ * 1e6));

	reg [BITS+LOG2DELAY-1:0] counter1 = 0;
	reg [BITS+LOG2DELAY-1:0] counter2 = 0;
	reg [BITS-1:0] outcnt;

	always @(posedge clk) begin
		counter1 <= counter1 + 1;
		outcnt <= counter1 >> LOG2DELAY;
		counter2 <= counter2 + {flash_io3_di, flash_io2_di, flash_io1_di, flash_io0_di};
	end
endmodule

[cliffordwolf]

Why in gods name is this a script pass!?!?!?

Hitting the refcount_wires_ assert in RTLIL::Module::add(RTLIL::Wire *wire). The value is 1 (rather than zero) but I don't understand why.

Because you are trying to add a wire while iterating over the list of wires. Do not do that.

 The output ports are showing up as input ports on the subcells.

I don't know what this means.

When do I need to run module->fixup_ports();

Whenever you made changes to the port indices so that there may be gaps, or indices may not be unique, or ports have no index assigned.

[mithro]

I've updated this pass quite a bit and it is getting closer to working (as you can see from the following screenshots);

However there are a bunch of things I'm still confused about;

• It seems I need to use a new wire to give to cell->setPort and then use module->connect to connect the cell port wire to an existing wire. Is this correct? When should you create new wires? Should every setPort call have it's own unique wire, or should I be able to give the same wire to multiple ports?

• When do I actually need to set port_input and port_output values on a wire? I assumed you would only need to set them when you are also setting port_id value? Do you need to set them when calling cell->setPort?

• module->connect(a, b) is suppose to be the same as doing an assign a = b; however it seems to be directional and inserting _$BUF_ objects?

• I'm still unclear when I should be calling fixup_ports()? Is it safe to modify multiple ports and then call fixup_ports() or do I need to call it after every modification (IE before making another modification)? What about when I'm adding new ports?

• Are there convenience functions for adding new ports (like the new_module_port function) that I'm missing or are you expected to do it manually?

• Where would be the correct place to add tests for this pass? tests/simple? tests/various?

• Shouldn't clean be removing the BUF cells and those hanging wires (diamonds with $1477 etc)?

• Is there currently a way to make log_assert actually cause a failure that will trigger in gdb automatically (like assert normally would), or should I be setting a breakpoint on it?

[cliffordwolf]

It seems I need to use a new wire to give to cell->setPort and then use module->connect to connect the cell port wire to an existing wire. Is this correct? When should you create new wires? Should every setPort call have it's own unique wire, or should I be able to give the same wire to multiple ports?

You don't "need to use a new wire". Unfortunately you don't tell how got the impression you'd need to.

You can just use cell->setPort with any existing signal.

When do I actually need to set port_input and port_output values on a wire? I assumed you would only need to set them when you are also setting port_id value? Do you need to set them when calling cell->setPort?

Each port wire must have port_input || port_output and port_id != 0. Each non-port wire must have !port_input && !port_output and port_id == 0. This has nothing to do at all with cell->setPort. I don't even know what kind of requirement in any direction one could dream up here.

Furthermore, port_id must be unique within a module and there must be no gaps (first port starting with index 1 since 0 is reserved for non-port wires).

module->connect(a, b) is suppose to be the same as doing an assign a = b; however it seems to be directional and inserting $BUF objects?

It is the same as assign a = b. assign a = b is directional, so I have no idea what you mean by "however it seems to be directional". module->connect(a, b) certainly does not insert a $_BUF_ object.

I'm still unclear when I should be calling fixup_ports()? Is it safe to modify multiple ports and then call fixup_ports() or do I need to call it after every modification (IE before making another modification)? What about when I'm adding new ports?

You can modify multiple ports and then just call fixup_ports() once.

As I said before: "Whenever you made changes to the port indices so that there may be gaps, or indices may not be unique, or ports have no index assigned."

So when you add a new port, and don't make sure that you have set up the port indices in a way so that they are continuous and unique then yes, of course of have to call fixup_ports(). If you manage the port indices yourself then of course no, you don't need to call fixup_ports().

Are there convenience functions for adding new ports (like the new_module_port function) that I'm missing or are you expected to do it manually?

You just add a wire (using addWire()) and then set port_id, port_input, port_output, upto, and start_offset to reflect what you want.

Where would be the correct place to add tests for this pass? tests/simple? tests/various?

yosys-tests

Shouldn't clean be removing the BUF cells and those hanging wires (diamonds with $1477 etc)?

There are no BUF cells. There are $_BUF_ cells and they are all removed by clean.

The BUF things you see in the show output are visualizations of direct wire-to-wire connections, not cells. The clean command tries to canonicalize wire names used in signals so that those BUF boxes are moved outside the active path, in the cases where this is possible. But it's not possible in all cases.

We had this conversation before.

Is there currently a way to make log_assert actually cause a failure that will trigger in gdb automatically (like assert normally would), or should I be setting a breakpoint on it?

I don't know what you are asking, since setting a breakpoint on log_assert is a way (the obvious way) to do that.

[nakengelhardt]

I'm unclear on what problem this PR was intended to solve, but since there hasn't been any activity in almost two years I'm assuming it isn't going to move forward, so I'm going to close it as part of our PR cleanup effort. Tristate support has been improved significantly in the meantime, in case that helps. If you're still having trouble, please file an issue with the problem!