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

nmigen generates invalid RTLIL with negative shifts #302

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

nmigen generates invalid RTLIL with negative shifts #302

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


Copy link

Issue by ZirconiumX
Saturday Jan 11, 2020 at 17:13 GMT
Originally opened as m-labs/nmigen#302

from nmigen import *
from nmigen.back import verilog

class Test(Elaboratable):
    def __init__(self):
        self.i = Signal(8)
        self.o = Signal(8)

    def elaborate(self, platform):
        m = Module()

        m.d.comb += self.o.eq(self.i << -3)

        return m



Traceback (most recent call last):
  File "", line 16, in <module>
  File "/usr/local/lib/python3.7/dist-packages/nmigen-0.2.dev13+g476ce15-py3.7.egg/nmigen/back/", line 77, in convert
    return _convert_rtlil_text(rtlil_text, strip_internal_attrs=strip_internal_attrs)
  File "/usr/local/lib/python3.7/dist-packages/nmigen-0.2.dev13+g476ce15-py3.7.egg/nmigen/back/", line 65, in _convert_rtlil_text
    raise YosysError(error.strip())
nmigen.back.verilog.YosysError: ERROR: Found error in internal cell \top.$3 ($sshl) at kernel/
  attribute \src ""
  cell $sshl $3
    parameter \Y_WIDTH 4'1011
    parameter \B_WIDTH 2'11
    parameter \B_SIGNED 1'1
    parameter \A_WIDTH 4'1000
    parameter \A_SIGNED 1'0
    connect \Y $2
    connect \B 3'101
    connect \A \i
Copy link

The underlying cause is that RTLIL never supported signed shifts, but erroneously did not reject them in the validator. After YosysHQ/yosys#1551 got merged, our generated RTLIL became invalid.

There are two possible fixes here:

  • Transparently translate signed shifts into a mux fed by a left shift and a right shift cell;
  • Reject shifts by signed values.

Any opinions?

Copy link

Honestly, I'd say that shifts by negative numbers definitely point to a bug. Really, you could get the equivalent of option 1 with a rotate operation, which I think would be clearer in semantics.

Copy link

Everyone thinks we should ban them, so let's just do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

No branches or pull requests

3 participants