Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Assignment to a Record with zero-width fields generates invalid Verilog #312

Closed
nmigen-issue-migration opened this issue Jan 24, 2020 · 3 comments
Closed

Assignment to a Record with zero-width fields generates invalid Verilog #312

nmigen-issue-migration opened this issue Jan 24, 2020 · 3 comments
Labels
backend:rtlil bug
Milestone
0.2

Comments

@nmigen-issue-migration
Copy link

Issue by jfng
Friday Jan 24, 2020 at 16:39 GMT
Originally opened as m-labs/nmigen#312

Repro:

from nmigen import *
from nmigen.back import rtlil, verilog


class Top(Elaboratable):
    def elaborate(self, platform):
        m = Module()

        rec = Record([
            ("a", 1),
            ("b", 0),
        ])

        m.d.comb += rec.eq(1)

        return m


if __name__ == "__main__":
    top = Top()
    print(verilog.convert(top))
    # print(rtlil.convert(top))

Verilog output:

/* Generated by Yosys 0.9+1706 (git sha1 2bda51ac, clang 9.0.0 -fPIC -Os) */

(* generator = "nMigen" *)
(* top =  1  *)
(* \nmigen.hierarchy  = "top" *)
module top();
  (* src = "repro.py:9" *)
  wire rec__a;
  (* src = "repro.py:9" *)
  wire [-1:0] rec__b;
  assign "" = "";
  assign rec__a = 1'h1;
endmodule

RTLIL output:

attribute \top 1
attribute \nmigen.hierarchy "top"
module \top
  attribute \src "repro.py:9"
  wire width 1 \rec__a
  attribute \src "repro.py:9"
  wire width 0 \rec__b
  wire width 1 $verilog_initial_trigger
  process $group_0
    assign \rec__a 1'0
    assign \rec__b 0'0
    assign { \rec__b \rec__a } 1'1
    assign $verilog_initial_trigger $verilog_initial_trigger
    sync init
      update $verilog_initial_trigger 1'0
  end
  connect \rec__b 0'0
end
@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Friday Jan 24, 2020 at 16:49 GMT

Hm, arguably this is a bug in Yosys, but I think we can just not generate assignments to zero width lvalues.

@whitequark whitequark added this to the 0.2 milestone Jan 31, 2020
@jfng
Copy link
Member

jfng commented Feb 21, 2020

A variant of this bug can still occur accross module boundaries.

Repro:

from nmigen import *
from nmigen.back import rtlil, verilog


class Top(Elaboratable):
    def elaborate(self, platform):
        m = Module()

        r1 = Record([("addr", 0), ("data", 8)])
        r2 = Record([("addr", 0), ("data", 8)])
        r3 = Record([("addr", 0), ("data", 8)])

        m.submodules.a = a = Module()
        m.submodules.b = b = Module()

        a.d.comb += r1.eq(0)
        m.d.comb += r2.eq(r1)
        b.d.comb += r3.eq(r2)

        return m


print(verilog.convert(Top()))
# print(rtlil.convert(Top()))

This time Yosys chokes with a parser error:

Traceback (most recent call last):
  File "repro.py", line 23, in <module>
    print(verilog.convert(Top()))
  File "/home/jf/src/nmigen/nmigen/back/verilog.py", line 78, in convert
    return _convert_rtlil_text(rtlil_text, strip_internal_attrs=strip_internal_attrs)
  File "/home/jf/src/nmigen/nmigen/back/verilog.py", line 66, in _convert_rtlil_text
    raise YosysError(error.strip())
nmigen.back.verilog.YosysError: ERROR: Parser error in line 37: syntax error

RTLIL output:

attribute \generator "nMigen"
attribute \nmigen.hierarchy "top.a"
module \a
  attribute \src "repro.py:9"
  wire width 8 output 1 \r1__data
  wire width 1 $verilog_initial_trigger
  process $group_0
    assign { } 0'0
    assign \r1__data 8'00000000
    assign { \r1__data { } } 8'00000000
    assign $verilog_initial_trigger $verilog_initial_trigger
    sync init
      update $verilog_initial_trigger 1'0
  end
end
attribute \generator "nMigen"
attribute \nmigen.hierarchy "top.b"
module \b
  attribute \src "repro.py:10"
  wire width 8 input 1 \r2__data
  attribute \src "repro.py:11"
  wire width 8 \r3__data
  process $group_0
    assign { } 0'0
    assign \r3__data 8'00000000
    assign { \r3__data { } } { \r2__data { } }
    sync init
  end
end
attribute \generator "nMigen"
attribute \top 1
attribute \nmigen.hierarchy "top"
module \top
  attribute \src "repro.py:9"
  wire width 8 \a_r1__data
  cell \a \a
    connect { } { }
    connect \r1__data \a_r1__data
  end
  attribute \src "repro.py:10"
  wire width 8 \b_r2__data
  cell \b \b
    connect { } { }
    connect \r2__data \b_r2__data
  end
  process $group_0
    assign { } 0'0
    assign \b_r2__data 8'00000000
    assign { \b_r2__data { } } { \a_r1__data { } }
    sync init
  end
end

@whitequark
Copy link
Member

@jfng That's a slightly different bug with a different cause--could you open another issue to track that one?

@jfng jfng mentioned this issue Mar 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend:rtlil bug
Milestone
0.2
Development

No branches or pull requests
3 participants
@whitequark @jfng @nmigen-issue-migration