My module computes some constants based on the bit widths of the signals. In my design I have several instances of this module. In simulation, each instance seems to get the correct constants. However, when converting to VHDL, there is only a single set of constants. Thus all instances get the constants computed for the first instance. Is there an easy workaround, or should I delve into the source code to fix this?
Show us the code …
from myhdl import always_seq, always_comb, intbv, Signal
def biquad(clk, reset, x, y, b0, b1, b2, a1, a2, fbw):
"""Direct form I digital biquad filter
Input: x
Output: y
Numerator coefficients: b0, b1, b2
Denominator coefficients: a1, a2
Fractional bit widths: (x, y, b, a)
"""
x_fbw, y_fbw, b_fbw, a_fbw = fbw
# Number of fractional bits to discard
B_LO = b_fbw + x_fbw - y_fbw
A_LO = a_fbw
width = y._nrbits
B_HI = B_LO + width
A_HI = A_LO + width
if B_LO < 0:
shift = -B_LO
B_LO = 0
else:
shift = 0
s1 = Signal(x.val)
s2 = Signal(x.val)
s3 = Signal(y.val)
s4 = Signal(y.val)
@always_comb
def assign():
y.next = s3
@always_seq(clk.posedge, reset=reset)
def logic():
s1.next = x
s2.next = s1
s4.next = s3
mult_b0 = intbv(b0 * x)[B_HI:B_LO].signed() << shift
mult_b1 = intbv(b1 * s1)[B_HI:B_LO].signed() << shift
mult_b2 = intbv(b2 * s2)[B_HI:B_LO].signed() << shift
mult_a1 = intbv(a1 * s3)[A_HI:A_LO].signed()
mult_a2 = intbv(a2 * s4)[A_HI:A_LO].signed()
b_side = intbv(mult_b0 + mult_b1 + mult_b2)[width:].signed()
a_side = intbv(mult_a1 + mult_a2)[width:].signed()
s3.next = intbv(b_side - a_side)[width:].signed()
return logic, assign
In the top-level module I instantiate three of these with the following fbw arguments: (23, 27, 10, 29), (27, 27, 31, 29), and (23, 27, 24, 29). When I convert to VHDL, I get
architecture MyHDL of filter is
constant A_LO: integer := 29;
constant shift: integer := 0;
constant A_HI: integer := 65;
constant B_HI: integer := 42;
constant width: integer := 36;
constant B_LO: integer := 6;
with three copies of the biquad logic, i.e. all of them get the constants computed for the first instance.
It would have been nice if you had also shown the top-level code containing the three calls …
Also, I am not familiar with this kind of filters.
Can I assume that bx and ax are <= 1.0 ?
What is the range of x and y?
Does your code simulate correctly?
Actually your biquad code doesn’t convert into correct VHDL (with myhdl 0.10.dev). It may look good at first sight, but GHDL complains on these lines:
mult_b0 := to_integer(shift_left(signed((b0 * x)(62-1 downto 30)), 0));
mult_b1 := to_integer(shift_left(signed((b1 * s1)(62-1 downto 30)), 0));
mult_b2 := to_integer(shift_left(signed((b2 * s2)(62-1 downto 30)), 0));
with;
Multiple markers at this line
- extraneous input ')' expecting ';'
- GHDL: ',' is expected instead of '('
- no viable alternative at input '('
- No matching subprogram was found.
I tried it because I was curious about your alternative coding style 
Ah yes, sorry. I didn’t have time to convert the top-level code to a minimum working example yet. The three calls and their arguments are as follows:
inv_b0 = Signal(intbv(0, -2**31, 2**31))
"""All b's and a's are of this type. The values are:
inv_b0 = 0x30da66
inv_b1 = 0xffcf3265
inv_a1 = 0x20000000
lp_b0 = 0x83371
lp_b1 = 0x1066e2
lp_b2 = 0x83371
lp_a1 = 0xc172a282
lp_a2 = 0x1e9590ee
ctrl_b0 = 0x14ad62d
ctrl_b1 = 0x56af
ctrl b2 = 0xfeb58080
ctrl_a1 = 0xc204080f
ctrl_a2 = 0x1e041851"""
in2_valid = Signal(bool(0))
state_reset = ResetSignal(0, active=1, async=True)
inv_in = Signal(intbv(0, -2**23, 2**23))
inv_out = Signal(intbv(0, -2**35, 2**35))
lp_in = Signal(intbv(0, -2**35, 2**35))
lp_out = Signal(intbv(0, -2**35, 2**35))
ctrl_in = Signal(intbv(0, -2**23, 2**23))
ctrl_out = Signal(intbv(0, -2**35, 2**35))
inverse = biquad(in2_valid, state_reset, inv_in, inv_out,
inv_b0, inv_b1, inv_b2, inv_a1, inv_a2,
(23, 27, 10, 29))
lowpass = biquad(in2_valid, state_reset, lp_in, lp_out,
lp_b0, lp_b1, lp_b2, lp_a1, lp_a2,
(27, 27, 31, 29))
controller = biquad(in2_valid, state_reset, ctrl_in, ctrl_out,
ctrl_b0, ctrl_b1, ctrl_b2, ctrl_a1, ctrl_a2,
(23, 27, 24, 29))
The MyHDL code simulates correctly. So far I did only a visual inspection of the VHDL code, and the constants were the first thing that caught my eye. I guess I need to rethink how I define the signal widths of the intermediate results.
What you really need is working fixbv class, which alas isn’t there yet. 
My idea is that the fractional width of the intermediate results only needs to one more than the fractional width of the final result? This would simplify the coding, I guess.
A working fixbv class would indeed simplify things. I’m not using any extra fractional bits for the intermediate results at the moment. Their fractional bit width is the same as that of the final result. It is usually different from the fractional bit widths of a, b, and the input, however. I will need to see what generates valid VHDL.
You can do this kind of computation by declaring mult_*, b_side, a_side as signals, assign them in a @always_comb statement and use them as you do (in logic()).
You can also try to declare mult_*, b_side, a_side as variables outside logic() and assign them in logic() with statements like mult_b0[:] = …
this is how I do this (without variables):
def widthr(v):
if v < 0:
# using signed numbers requires double
tv = -v * 2
else:
# unsigned
tv = v
if tv == 0:
raise ValueError("O-value has no width")
elif tv == 1:
return 1
else:
exp = math.ceil(math.log(tv, 2))
if math.pow(2, exp) == tv:
exp += 1
r = int(exp)
print(v, r)
return r
def biquad_alt3(clk, reset, x, y, b, a, fbw):
"""Direct form I digital biquad filter
Input: x
Output: y
tuple of Numerator coefficients: [b0, b1, b2]
tuple of Denominator coefficients: [a1, a2]
tuple of Fractional bit widths: (x, y, bi, ai)
"""
x_fbw, y_fbw, b_fbw, a_fbw = fbw
assert x_fbw + b_fbw > y_fbw
b0 = int(b[0] * 2 ** b_fbw)
b1 = int(b[1] * 2 ** b_fbw)
b2 = int(b[2] * 2 ** b_fbw)
a1 = int(a[0] * 2 ** a_fbw)
a2 = int(a[1] * 2 ** a_fbw)
XLEN = len(x)
YLEN = len(y)
XB0LEN = XLEN + widthr(b0)
XB1LEN = XLEN + widthr(b1)
XB2LEN = XLEN + widthr(b2)
YA1LEN = YLEN + widthr(a1)
YA2LEN = YLEN + widthr(a2)
xd1, xd2 = [Signal(intbv(0, -2 ** (XLEN - 1), 2 ** (XLEN - 1))) for _ in range(2)]
yd1, yd2 = [Signal(intbv(0, -2 ** (YLEN - 1), 2 ** (YLEN - 1))) for _ in range(2)]
xb0 = Signal(intbv(0, -2 ** (XB0LEN - 1), 2 ** (XB0LEN - 1)))
xd1b1 = Signal(intbv(0, -2 ** (XB1LEN - 1), 2 ** (XB1LEN - 1)))
xd2b2 = Signal(intbv(0, -2 ** (XB2LEN - 1), 2 ** (XB2LEN - 1)))
yd1a1 = Signal(intbv(0, -2 ** (YA1LEN - 1), 2 ** (YA1LEN - 1)))
yd2a2 = Signal(intbv(0, -2 ** (YA2LEN - 1), 2 ** (YA2LEN - 1)))
ly = Signal(intbv(0, -2 ** YLEN, 2 ** YLEN))
@always_comb
def comb1():
xb0.next = x * b0
xd1b1.next = xd1 * b1
xd2b2.next = xd2 * b2
yd1a1.next = yd1 * a1
yd2a2.next = yd2 * a2
@always_comb
def comb2():
ly.next = (xb0[:x_fbw + b_fbw - y_fbw - 1].signed() + xd1b1[:x_fbw + b_fbw - y_fbw - 1].signed() + xd2b2[:x_fbw + b_fbw - y_fbw - 1].signed()) \
- (yd1a1[:a_fbw - 1].signed() + yd2a2[:a_fbw - 1].signed())
@always_comb
def comb3():
y.next = ly[:1]
@always_seq(clk.posedge, reset)
def synch():
xd1.next = x
xd2.next = xd1
yd1.next = ly[:1]
yd2.next = yd1
return comb1, comb2, comb3, synch
def top_biquad(in2_valid, state_reset, inv_in, inv_out, lp_in, lp_out, ctrl_in, ctrl_out):
inverse = biquad_alt3(in2_valid, state_reset, inv_in, inv_out,
(1.0, 1.0, 1.0), (1.0, 1.0),
(23, 27, 10, 29))
lowpass = biquad_alt3(in2_valid, state_reset, lp_in, lp_out,
(1.0, 1.0, 1.0), (1.0, 1.0),
(27, 27, 31, 29))
controller = biquad_alt3(in2_valid, state_reset, ctrl_in, ctrl_out,
(1.0, 1.0, 1.0), (1.0, 1.0),
(23, 27, 24, 29))
return inverse, lowpass, controller
if __name__ == '__main__':
in2_valid = Signal(bool(0))
state_reset = ResetSignal(0, active=1, async=True)
inv_in = Signal(intbv(0, -2 ** 23, 2 ** 23))
inv_out = Signal(intbv(0, -2 ** 35, 2 ** 35))
lp_in = Signal(intbv(0, -2 ** 35, 2 ** 35))
lp_out = Signal(intbv(0, -2 ** 35, 2 ** 35))
ctrl_in = Signal(intbv(0, -2 ** 23, 2 ** 23))
ctrl_out = Signal(intbv(0, -2 ** 35, 2 ** 35))
toVHDL(top_biquad, in2_valid, state_reset, inv_in, inv_out, lp_in, lp_out, ctrl_in, ctrl_out)
You see that I now need three @always_comb because MyHDL doesn’t allow using outputs as inputs inside an @always_comb.
I have removed that restriction in my local MyHDL code, but this is support for the official distribution.
The variable approach suggested by @DrPi can get away with just the @always_seq but I leave that exercise to him (or you?)
Also I just used 1.0 for every coefficient, you will put in the correct values?
I am not sure whether it works, as I haven’t simulated? SO I shall have no pie 
Thanks for your tips. I ended up taking @DrPi’s advice:
from myhdl import always_seq, always_comb, intbv, modbv, Signal
def biquad(clk, reset, x, y, b0, b1, b2, a1, a2, fbw):
"""Direct form I digital biquad filter
Input: x
Output: y
Numerator coefficients: b0, b1, b2
Denominator coefficients: a1, a2
Fractional bit widths: (x, y, b, a)
"""
## Constants
x_fbw, y_fbw, b_fbw, a_fbw = fbw
# Number of fractional bits to discard
B_LO = b_fbw + x_fbw - y_fbw
A_LO = a_fbw
width = len(y)
B_HI = B_LO + width
A_HI = A_LO + width
if B_LO < 0:
shift = -B_LO
B_LO = 0
else:
shift = 0
# Intermediate result range
bx_min, bx_max = (-2**(width - 1 - shift), 2**(width - 1 - shift))
ay_min, ay_max = (-2**(width - 1), 2**(width - 1))
full_bx_maxexp = len(b0) + len(x) - 1
full_ay_maxexp = len(a1) + len(y) - 1
full_bx_min, full_bx_max = (-2**full_bx_maxexp, 2**full_bx_maxexp)
full_ay_min, full_ay_max = (-2**full_ay_maxexp, 2**full_ay_maxexp)
## Internal signals
s1, s2 = [Signal(x.val) for _ in range(2)]
s3, s4 = [Signal(y.val) for _ in range(2)]
full_bx = [Signal(intbv(0, full_bx_min, full_bx_max)) for _ in range(3)]
full_ay = [Signal(intbv(0, full_ay_min, full_ay_max)) for _ in range(2)]
bx = [Signal(intbv(0, bx_min, bx_max)) for _ in range(3)]
ay = [Signal(intbv(0, ay_min, ay_max)) for _ in range(2)]
sums = Signal(modbv(0)[width:])
@always_comb
def multiply():
full_bx[0].next = b0 * x
full_bx[1].next = b1 * s1
full_bx[2].next = b2 * s2
full_ay[0].next = a1 * s3
full_ay[1].next = a2 * s4
@always_comb
def add():
sums.next = bx[0] + bx[1] + bx[2] - ay[0] - ay[1]
@always_comb
def assign():
y.next = s3
bx[0].next = full_bx[0][B_HI:B_LO].signed() << shift
bx[1].next = full_bx[1][B_HI:B_LO].signed() << shift
bx[2].next = full_bx[2][B_HI:B_LO].signed() << shift
ay[0].next = full_ay[0][A_HI:A_LO].signed()
ay[1].next = full_ay[1][A_HI:A_LO].signed()
@always_seq(clk.posedge, reset=reset)
def sync():
s1.next = x
s2.next = s1
s3.next = sums.signed()
s4.next = s3
return multiply, add, assign, sync
This produces valid VHDL and simulates ok. However, I’m still left with a single set of constants, so all instances get the fractional bit widths of the first instance. This happens with @josyb’s code as well.
can you give me a valid set of constant?
The Python code computes the constants just fine. In the VHDL file generated by your code, the constants are as follows:
architecture MyHDL of top_biquad is
constant b0: integer := 2**10;
constant x_fbw: integer := 23;
constant a2: integer := 2**29;
constant a_fbw: integer := 29;
constant b1: integer := 2**10;
constant b_fbw: integer := 10;
constant a1: integer := 2**29;
constant y_fbw: integer := 27;
constant b2: integer := 2**10;
There should be a set of these constants for each instance.
Or do you mean floating-point values for b and a? These should be ok:
inv b0: 3126.599609
inv b1: -3123.401367
inv a1: 1.000000
lp b0: 0.000250
lp b1: 0.000501
lp b2: 0.000250
lp a1: -1.954756
lp a2: 0.955758
ctrl b0: 1.292331
ctrl b1: 0.001323
ctrl b2: -1.291008
ctrl a1: -1.937008
ctrl a2: 0.938000
In my application they are not constants, but 32-bit registers that can be overwritten.
I’ll try this later tonight.
I upgraded to MyHDL 0.10 and the new API. Browsing through the VHDL, it seems to have fixed this issue. No constants are defined in the VHDL file anymore. Instead, the computed values are now hardcoded into wherever they are used, and each instance gets its own set of values.
The upgrade caused an unrelated major nuisance, though: I get “ToVHDLWarning: Previously used name being reused” warnings every time I rerun the conversion. In simulation, the blocks are renamed on every run, so that it is necessary to manually add all signals in GTKWave every time I run the simulation. After the latest simulation, the blocks are named biquad0_0_1_2_3_4_5, biquad1_0_1_2_3_4_5, and biquad2_0_1_2_3_4_5. (I added this as another issue, but I mention it here as well, since the code is already in this thread.)