ghdl / ghdl-yosys-plugin

VHDL synthesis (based on ghdl)
GNU General Public License v3.0
295 stars 32 forks source link

High impedance assignment translates to 1'x #170

Open DanielG opened 1 year ago

DanielG commented 1 year ago

When trying to assign a pin to HiZ I get a hard low drive instead. Reproducer below. I dub this bug "resisting high impedance" :)

ent.vhdl:

library ieee;
use ieee.std_logic_1164.all;

entity ent is
  port (p : out std_ulogic);
end entity;

architecture a of ent is
begin
  p <= 'Z';
end architecture;

GHDL synth keeps the 'Z':

$ ghdl synth -e ent.vhdl
library ieee;
use ieee.std_logic_1164.all;
entity ent is
  port (
    p: out std_ulogic
  );
end entity;

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

architecture rtl of ent is
  signal wrap_p: std_logic;
  constant n1_o : std_logic := 'Z';  --- <-- Hi-Z still here
begin
  p <= wrap_p;
  wrap_p <= n1_o;
end rtl;

but ghdl-yosys-plugin seems to translate it to 1'x:

$ ghdl import ent.vhdl && ghdl make ent && yosys -m ghdl -p 'ghdl ent; write_rtlil'
[...]
# Generated by Yosys 0.12 (git sha1 2156e20)
autoidx 1
module \ent
  wire output 1 \p
  connect \p 1'x
end

Versions (all using my Debian packaging):

ghdl-yosys-plugin 0.0~git20211127.09a32cd-2~bpo11+1 ghdl 2.0.0+dfsg-1~bpo11+1 yosys 0.12-1~bpo11+1

tgingold commented 1 year ago

Yes, but you can the same with a verilog input:

module m (output v); assign v = 1'bz; endmodule

autoidx 1 attribute \cells_not_processed 1 attribute \src "t.v:1.1-3.10" module \m attribute \src "t.v:1.18-1.19" wire output 1 \v connect \v 1'x end

So it is probably a limitation of yosys.

DanielG commented 1 year ago

I managed to work around this using 'X' when false else 'Z' instead. This generates a $tribuf which seems to work. Perhaps the ghdl plugin should also use this pattern? I'd be happy to report a yosys bug too though.

DanielG commented 1 year ago

Looks like there's an open yosys bug for this: https://github.com/YosysHQ/yosys/issues/511

DanielG commented 1 year ago

Here's a gdb session log for the yosys reproducer above. Looks like internally the constant does have the Sz value but the rtlil backend dumps it as 'x' in this code because is_fully_undef returns true:

void RTLIL_BACKEND::dump_const(...)
        [...]
    if (data.is_fully_undef()) {
        f << "x";
    } else {
        for (int i = offset+width-1; i >= offset; i--) {
            log_assert(i < (int)data.bits.size());
            switch (data.bits[i]) {
            case State::S0: f << stringf("0"); break;
            case State::S1: f << stringf("1"); break;
            case RTLIL::Sx: f << stringf("x"); break;
            case RTLIL::Sz: f << stringf("z"); break;
            case RTLIL::Sa: f << stringf("-"); break;
            case RTLIL::Sm: f << stringf("m"); break;
            }
        }
    }
2. Executing RTLIL backend.
Output filename: <stdout>
# Generated by Yosys 0.12 (git sha1 2156e20)
autoidx 1
attribute \cells_not_processed 1
attribute \src "/home/dxld/share/elec/nppd/fpga/repro/resisting-high-impedance/ent.v:1.1-3.10"
module \ent
  attribute \src "/home/dxld/share/elec/nppd/fpga/repro/resisting-high-impedance/ent.v:1.20-1.21"
  wire output 1 \v

Breakpoint 4, Yosys::RTLIL_BACKEND::dump_const (f=..., data=..., width=1, 
    offset=0, autoint=true) at backends/rtlil/rtlil_backend.cc:34
34  {
(gdb) tbreak RTLIL::Const::is_fully_undef
Temporary breakpoint 5 at 0x5555556ad9c4: file kernel/rtlil.cc, line 372.
(gdb) c
Continuing.

Temporary breakpoint 5, Yosys::RTLIL::Const::is_fully_undef (
    this=this@entry=0x7fffffffd648) at kernel/rtlil.cc:372
warning: Source file is more recent than executable.
372     cover("kernel.rtlil.const.is_fully_undef");
(gdb) p bits
$4 = std::vector of length 1, capacity 1 = {Sz}
(gdb) n
374     for (const auto &bit : bits)
(gdb) 
375         if (bit != RTLIL::State::Sx && bit != RTLIL::State::Sz)
(gdb) p bit
$5 = (const State &) @0x555555ebd7c0: Sz
DanielG commented 1 year ago

Indeed the write_verilog output looks correct. I'm pretty sure something in the toolchain is miscompiling this as I can observe the bus conflicts on my Oscilloscope :)

tgingold commented 1 year ago

Ok, so that's just a bug with write_rtlil.

Do not hesitate to report the issue once you have figured out it.