YosysHQ / yosys

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

Wired-or (wor) wires generate $or / $reduce_or cells in output #4389

Open jswrightoc opened 4 months ago

jswrightoc commented 4 months ago

Version

Yosys 0.41+24 (git sha1 75f01ccee, g++ 8.3.0-6 -fPIC -Os)

On which OS did this happen?

Linux

Reproduction Steps

Problem Usage of wired-or (wor) connections can result in $or or $reduce_or intermediate cells being generated in the output, which is not handled by place-and-route tools. For example, using nextpnr-ice40: ERROR: cell type '$reduce_or' is unsupported (instantiated as '$auto$hierarchy.cc:1398:execute$745')

Test Source

module ice40_main
  (
    input  TEST_0,
    output TEST_1,
  );
  wor test_wire_w;
  assign test_wire_w = TEST_0;
  assign TEST_1 = test_wire_w;
endmodule

Yosys Script

read_verilog src/ice40.v
synth_ice40 -top ice40_main

Results

yosys> stat

3. Printing statistics.

=== ice40_main ===

   Number of wires:                  3
   Number of wire bits:              3
   Number of public wires:           3
   Number of public wire bits:       3
   Number of ports:                  2
   Number of port bits:              2
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     $reduce_or                      1

At this point, running opt will eliminate the intermediate $reduce_or cell in this example (indeed, the first pass of opt_expr takes care of it), though I suspect this is only masking the underlying problem. Simply declaring test_wire_w as wire works too, although that defeats the purpose of using wor to allow assignment from multiple sources, e.g. address/data buses.

This is a trivial example; the results of more complex designs vary widely. Some may still generate erroneous output even after opt (I'm afraid I don't have any good examples I can share at the moment). Other designs may produce normal output even with several instances of wired-or connections.

Versions I have tested this on several versions going all the way back to yosys-0.9 and, as far as I can tell, this source will always reproduce the undesired behavior.

More Testing I have another more complex test case I was working on that would generate successful output for a range of versions, but only if the wire name was greater than the module name (as if evaluated with strcmp) whose input was assigned to the wire. This may or may not be peripherally related to the first described problem; I discovered this on the path to generating a minimal reproducible example. Here are some truncated examples.

This would fail (foo_w less than foo_x):

wor [9:0]  foo_w;
module mem_core foo_x
(
  .raddr_i(foo_w),
);

This would not fail (foo_w greater than foo_v):

wor [9:0]  foo_w;
module mem_core foo_v
(
  .raddr_i(foo_w),
);

The described name-dependent behavior begins with https://github.com/YosysHQ/yosys/commit/7ae4041 (inclusive) and ends with https://github.com/YosysHQ/yosys/commit/8902fc94b (exclusive). I can share more information on this if it is found to be relevant to the fundamental problem.

Expected Behavior

No intermediate cells present in output of yosys.

Actual Behavior

Intermediate cells including $or and $reduce_or are sometimes present in the output of yosys.

whitequark commented 4 months ago

That's pretty weird! I would have expected synth_ice40 to lower wor before lowering $reduce_or.

The described name-dependent behavior begins with https://github.com/YosysHQ/yosys/commit/7ae4041 (inclusive) and ends with https://github.com/YosysHQ/yosys/commit/8902fc94b (exclusive).

This is probably an artefact of how clean works, which is weird but fairly well known.

jswrightoc commented 4 months ago

For what it's worth, the same thing happens using synth instead of synth_ice40.

yosys> read_verilog src/ice40.v

1. Executing Verilog-2005 frontend: ice40_test1.v
Parsing Verilog input from `ice40_test1.v' to AST representation.
Generating RTLIL representation for module `\ice40_main'.
Successfully finished Verilog frontend.

yosys> synth

2. Executing SYNTH pass.

[numerous messages]

2.25. Printing statistics.

=== ice40_main ===

   Number of wires:                  3
   Number of wire bits:              3
   Number of public wires:           3
   Number of public wire bits:       3
   Number of ports:                  2
   Number of port bits:              2
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     $reduce_or                      1

2.26. Executing CHECK pass (checking for obvious problems).
Checking module ice40_main...
Found and reported 0 problems.

yosys>

Looks like the $reduce_or remains as a useless appendage, which may explain why opt gets rid of it. yosys

This is probably an artefact of how clean works, which is weird but fairly well known.

Yeah, from what I've read it's one of those operations whose goals aren't easily defined, but there are a lot of things that must be done. It was just kind of crazy that it was name-dependent - I have a working design, then I go and rename some things so that it makes more sense and, boom! It explodes in my face. :)

whitequark commented 4 months ago

I have a working design, then I go and rename some things so that it makes more sense and, boom! It explodes in my face. :)

Personally I think it's ridiculous that we have this working the way it does, but it also proved rather difficult to fix.