opt_clean reverses some connections

[georgerennie]

Steps to reproduce the issue

read_verilog <<EOT
module top(
	input wire in,
	output wire out1, out2
);

wire int1, int2;
assign int1 = in;
assign out1 = in;
assign out2 = int2;

endmodule
EOT

dump
opt_clean
dump

Expected behavior

Both dump outputs should contain

  connect \int1 \in
  connect \out1 \in
  connect \out2 \int2

Actual behavior

Second dump output instead contains

  connect \int2 \out2
  connect \int1 \in
  connect \out1 \in

Notably, the connection between out2 and int2 has been reversed. I played around with connecting a few different types of signals and only saw this occur for an undriven signal driving an output wire.

[nakengelhardt]

Does this cause a problem? Semantically it seems identical to me, two undriven wires aliasing each other.

[georgerennie]

Not necessarily but it can lead to some slightly confusing behaviour, e.g. Sby reports the output net as being undriven, rather than the source (I am not at my computer right now to check which yosys command that corresponds to). I'm happy for the issue to be closed if its not considered an issue.

[georgerennie]

Not necessarily but it can lead to some slightly confusing behaviour, e.g. Sby reports the output net as being undriven, rather than the source (I am not at my computer right now to check which yosys command that corresponds to). I'm happy for the issue to be closed if its not considered an issue.

…

On Mon, 1 Aug 2022, 15:53 N. Engelhardt, ***@***.***> wrote: Does this cause a problem? Semantically it seems identical to me, two undriven wires aliasing each other. — Reply to this email directly, view it on GitHub <#3426 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEVCE6V6KYQESHMIS7IBSUDVW7QIBANCNFSM5464ASWA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[KatCe]

We have built a pass that analyses data flows in designs. The reversing of connections prevents our pass from being able to determine the data flow direction between these two signals.

[nakengelhardt]

There won't be any data flowing through an undriven wire, though, so does that actually have relevant effects on the result? As a workaround, you could use something like setundef -undriven -undef before the first call to opt_clean to drive those wires with Xs.

[KatCe]

We saw it happening with driven wires after optimizations.

[povik]

Based on knowledge of the opt_clean code I can confirm that when you have a pair of connected wires, it can happen that the driver is switched from one of the wires to the other, and the direction of the connection is reversed.

[nakengelhardt]

But in that case all users of the signal should also be connected to the chosen wire, no?

(By the way I'm not saying this is all perfect as it is, it's already a long-standing goal to redesign opt_clean from the ground up: #2165, I'm just trying to differentiate if it's a "this representation isn't ideal for all use cases" issue or a "my design is mis-synthesized" critical bug.)

[povik]

But in that case all users of the signal should also be connected to the chosen wire, no?

Yes, except for an output port, which is represented by another wire, but in that case the connections between wires are appropriately reversed. So this is a representation issue at worst.

[georgerennie]

From my memory of this, I suspect I hit it for similar reasons to Katharina because I was trying to do the same type of thing. I believe this shouldn't cause mis-synthesis as from the perspective of i/o and assertions etc the behaviour shouldn't change due to this afaik.

The main annoyance is when you are doing analysis where you want to cut the driver of a signal or something by name, so the order/direction of connections matters. I haven't been able to create a case where the order is reversed other than when one of the signals is undriven (which can happen in e.g. formal testbenches where an undriven wire is used as an oracle), but below is a similar example that can be annoying for this type of analysis.

#! /usr/bin/env yosys

read_verilog <<EOT
`default_nettype none
module top(
	input wire in,
	output wire out1, out2, out3
);

wire int1, int2;
assign int1 = in;
assign int2 = int1;

assign out1 = int1;
assign out2 = int2;
assign out3 = out1 | out2;

endmodule
EOT

dump
opt_clean
dump
cutpoint top/out1
dump

The result of this is that instead of maintaining the order between those signals, they all get driven directly by in. This means when you cutpoint out1, only that one signal gets cutpointed, but out2 and out3 are still driven by in directly.

  cell $or $or$<<EOT:13$1
    ...
    connect \A \in
    connect \B \in
    connect \Y \out3
  end

  connect \int2 \in
  connect \int1 \in
  connect \out2 \in
  connect $auto$cutpoint.cc:91:execute$2 \in
  connect \out1 $auto$rtlil.cc:3274:Anyseq$4

I feel like this makes sense and is expected under the current behaviour of yosys (you can only really rely on the passes to maintain the behaviour seen at i/o and checkers), but there being wires in the transformed module with the same name as in the original but different structural relations can be confusing. From what I remember the main place I hit this type of thing was where prep would be run on a design with the intent of just lowering it to a netlist before modifying it for analysis, and it would produce confusing results due to this.

[nakengelhardt]

I see, yes in that context it is cryptic why that is happening to the point that I would consider it a bug!

[KatCe]

I agree that the optimizations shown in the example by @georgerennie are are making certain things like signal abstractions more difficult, especially if you want to do that automatically.

@nakengelhardt The switching of the direction of connections/assignments is problematic for data flow analysis. We did not observe broken connections.