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.

Sign up for GitHub

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

Signals with non power of two maximum value #39

Closed
rroohhh opened this issue Mar 4, 2019 · 4 comments
Closed

Signals with non power of two maximum value #39

rroohhh opened this issue Mar 4, 2019 · 4 comments
Labels
question

Comments

@rroohhh
Copy link
Contributor

rroohhh commented Mar 4, 2019

Currently the maximum value of a Signal specified using the max parameter is rounded up to the next power of two.
Because of this something like the following fails with IndexError: Cannot end slice 10 bits into 9-bit value

class Test:
    def __init__(self):
        self.ctr = Signal(max=9)
        self.out = Signal(9)
        self.input = Signal()

    def elaborate(self, platform):
        m = Module()
        m.d.sync += self.out.part(self.ctr, 1).eq(self.input)

        with m.If(self.ctr == 9):
            m.d.sync += self.ctr.eq(0)
        with m.Else():
            m.d.sync += self.ctr.eq(self.ctr + 1)
        
        return m

While I understand the cause for this error, I don't think this is the best solution possible.
I can think of two possible ways to improve:

  1. Adding a new kind of Signal that supports non power of two maximum values (and guarantees that it is not excceded). Guaranteeing this may come with a high cost, but would also simplify some code, like this counter.
  2. Adding a new parameter to the Signal constructor which lets the user assure the specified maximum value is never exceded. Something like this could also be added to the new kind of Signal proposed.
@whitequark
Copy link
Contributor

What about using Array(self.out)[self.ctr]? Arrays are defined to clamp the index to the maximum possible.

@rroohhh
Copy link
Contributor Author

rroohhh commented Mar 4, 2019 •
edited

While this is a nice solution for the .part() case, there are similar cases where this issue is hidden.
For example something like self.out | (1 << self.ctr) makes the immediate wire wider than needed. If they will get optimized away by synthesis that would be fine, but I am not sure if the synthesis tool is always able to optimize cases like this.

@whitequark
Copy link
Contributor

If they will get optimized away by synthesis that would be fine, but I am not sure if the synthesis tool is always able to optimize cases like this.

They will. If you have an especially braindead synthesis tool that can't do that, just use Yosys to preprocess the gateware by running architecture-independent optimizations before feeding it to that tool.

@rroohhh
Copy link
Contributor Author

rroohhh commented Mar 4, 2019

Ok then the Array(self.out)[self.ctr] solution fully solves my concerns, thanks.

@rroohhh rroohhh closed this as completed Mar 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@whitequark @rroohhh