From 2310d0eba5ef940042bcded3bea7767ed7e8ae2c Mon Sep 17 00:00:00 2001 From: Julian Kelly Date: Thu, 25 Feb 2016 12:32:40 -0800 Subject: [PATCH] Fix 'from address too close' behavior in dac.py Improve documentation for jump_table in dac.py Add test_dac Change dac build 15 offsets: *JT_MIN_FROM_ADDR_SPACING -> JT_MIN_CLK_CYCLES_BETWEEN_OPCODES *JT_MIN_CLK_CYCLES_BETWEEN_OPCODES: 2->4 in line with GHZDAC8 specs *JT_MIN_FROM_ADDR remove offset *JT_MIN_END_ADDR remove offset Fixes #323 *Add test, error for long jump tables Review: @maffoo --- fpgalib/dac.py | 84 +++++++++++++++++++++----- fpgalib/jump_table.py | 22 +++++-- fpgalib/test/test_dac.py | 126 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 21 deletions(-) create mode 100644 fpgalib/test/test_dac.py diff --git a/fpgalib/dac.py b/fpgalib/dac.py index 7823edf7..2de99d19 100644 --- a/fpgalib/dac.py +++ b/fpgalib/dac.py @@ -1172,6 +1172,23 @@ class DAC_Build15(DAC_Build8): 19 d[1] ... 33 d[15] + + Table documenting how each op code can move the sram, jump table index + + OP | BEHAVIOR + ------------------------------------------------------------------------ + IDLE | increment sram_idx and increment jt_idx + NOP | increment sram_idx and increment jt_idx + CHECK | move sram_idx and jt_idx OR increment sram_idx and jt_idx + JUMP | move sram_idx and jt_idx + CYCLE | move sram_idx and jt_idx OR increment sram_idx and jt_idx + END | no next entry + + Attributes: + increment_ops (list[jump_table.Operation]): Jump table op codes that can + increment the sram index, jump table index + jump_ops (list[jump_table.Operation]): Jump table op codes that can + jump the sram index, jump table index """ HAS_JUMP_TABLE = True @@ -1181,21 +1198,20 @@ class DAC_Build15(DAC_Build8): SRAM_WRITE_PKT_LEN = 256 SRAM_WRITE_DERPS = SRAM_LEN // SRAM_WRITE_PKT_LEN - JUMP_TABLE_LEN = 528 + JUMP_TABLE_PACKET_LEN = 528 NUM_COUNTERS = 4 COUNTER_BYTES = 4 IDLE_BITS = 15 JUMP_TABLE_ENTRY_BYTES = 8 - JUMP_TABLE_COUNT = (JUMP_TABLE_LEN - (NUM_COUNTERS * COUNTER_BYTES) - ) / JUMP_TABLE_ENTRY_BYTES + JUMP_TABLE_COUNT = 64 JT_FROM_ADDR_OFFSET = -2 JT_END_ADDR_OFFSET = JT_FROM_ADDR_OFFSET - 1 JT_IDLE_OFFSET = -0 JT_IDLE_MIN = 0 - JT_IDLE_OFFSET JT_IDLE_MAX = 2**IDLE_BITS - 1 - JT_IDLE_OFFSET - JT_MIN_FROM_ADDR_SPACING = 2 - JT_MIN_FROM_ADDR = JT_MIN_FROM_ADDR_SPACING - JT_FROM_ADDR_OFFSET - JT_MIN_END_ADDR = JT_MIN_FROM_ADDR_SPACING - JT_END_ADDR_OFFSET + JT_MIN_CLK_CYCLES_BETWEEN_OPCODES = 4 + JT_MIN_FROM_ADDR = JT_MIN_CLK_CYCLES_BETWEEN_OPCODES + JT_MIN_END_ADDR = JT_MIN_CLK_CYCLES_BETWEEN_OPCODES JT_MIN_TO_ADDR = 0 JT_MAX_FROM_ADDR = SRAM_LEN // 4 + JT_FROM_ADDR_OFFSET JT_MAX_END_ADDR = SRAM_LEN // 4 + JT_END_ADDR_OFFSET @@ -1205,6 +1221,15 @@ class DAC_Build15(DAC_Build8): MONITOR_0 = 5 MONITOR_1 = 10 + increment_ops = ( + jump_table.IDLE, jump_table.NOP, jump_table.CHECK, + jump_table.CYCLE + ) + + jump_ops = ( + jump_table.CHECK, jump_table.JUMP, jump_table.CYCLE + ) + @classmethod def convert_from_address(cls, x_ns): """ Convert a from address from ns to SRAM index. @@ -1430,7 +1455,7 @@ def make_jump_table_entry(cls, name, arg): :param str name: one of: END, NOP, IDLE, CYCLE, JUMP, CHECK, RAMP :param list[int] arg: arguments for the op. :return: list of jump table entries - :rtype: list[jump_table.JumpEntry] + :rtype: jump_table.JumpEntry """ if name == 'CHECK': raise NotImplementedError("Check not implemented yet") @@ -1477,23 +1502,52 @@ def make_jump_table_entry(cls, name, arg): def make_jump_table(cls, jt_entries, counters=None, start_address_ns=0): """Make a jump table out of the given entries and counters. + We verify that jump table commands are not executed too frequently. We + check when our current command executes, and compare to execution of + the next command. The next command depends on the specifics of the + current command, arguments and conditions. See attributes increment_ops, + jump_ops. + :param list[jump_table.JumpEntry] jt_entries: JT entries :param list[int] counters: counter values, or None for all 0s :param int start_address_ns: SRAM start address, in ns :return: jump table object :rtype: jump_table.JumpTable """ - for i, a in enumerate(jt_entries): - for j, b in enumerate(jt_entries[i+1:]): - if abs(a.from_addr - b.from_addr) < cls.JT_MIN_FROM_ADDR_SPACING: - raise ValueError( - "Entries {}({}) and {}({}) have from addrstoo close together.".format( - i, a.operation, i+j+1, b.operation) - ) + + # jt_entries does not include initial NOP; see #325 + if len(jt_entries) + 1 > cls.JUMP_TABLE_COUNT: + raise ValueError("Jump table too long: {} + starting NOP > {}" + .format(len(jt_entries), cls.JUMP_TABLE_COUNT)) + + # no two opcodes executed within JT_MIN_CLK_CYCLES_BETWEEN_OPCODES + for k, jt_entry in enumerate(jt_entries): + # first pass we check all increment cases + if isinstance(jt_entry.operation, cls.increment_ops): + sram_idx = jt_entry.from_addr + next_jt_idx = k + 1 + next_from_addr = jt_entries[next_jt_idx].from_addr + if ((next_from_addr - sram_idx) < + cls.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES): + raise ValueError("Entry {} and entry it increments to " + "executed too closely in time" + .format(jt_entry)) + # second pass we check all jump cases + if isinstance(jt_entry.operation, cls.jump_ops): + sram_idx = jt_entry.to_addr + # take off 1 as first entry is always NOP (self.toString) + next_jt_idx = jt_entry.operation.jump_index - 1 + next_from_addr = jt_entries[next_jt_idx].from_addr + if ((next_from_addr - sram_idx) < + cls.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES): + raise ValueError("Entry {} and entry it jumps to executed " + "too closely in time".format(jt_entry)) + return jump_table.JumpTable( start_addr=cls.convert_to_address(start_address_ns), jumps=jt_entries, - counters=counters + counters=counters, + packet_len=cls.JUMP_TABLE_PACKET_LEN, ) @classmethod diff --git a/fpgalib/jump_table.py b/fpgalib/jump_table.py index 3cb1ead1..549509eb 100644 --- a/fpgalib/jump_table.py +++ b/fpgalib/jump_table.py @@ -27,6 +27,8 @@ def __init__(self, from_addr, to_addr, operation): :param int to_addr: the to address (should be 0 if not used) :param Operation operation: the operation """ + + # TODO: Why is this -1 hard coded? if abs(from_addr - to_addr) - 1 == 0: raise ValueError( "from_addr: {} and to_addr: {} are too close".format( @@ -114,6 +116,8 @@ class CHECK(Operation): which_daisy_bit (int): Which daisy bit to check. Need to explain how the numbering works. jump_index (int): Index of jump entry to activte after the check. + Note: jump_table gets an extra NOP start entry at run time in + self.toString, so first user entry is index 1. bit_state (bool): Selects whether to fire on 0 or 1. Need to explain more. @@ -153,7 +157,8 @@ class JUMP(Operation): Attributes: jump_index (int): Index of jump table entry to activate after - this one. + this one. Note: jump_table gets an extra NOP start entry at run time + in self.toString, so first user entry is index 1. """ NAME = "JUMP" @@ -195,8 +200,9 @@ class CYCLE(Operation): """Cycle back to another SRAM address. Attributes: - jump_index (int): Index of jump table entry to activate after firing - this one. + jump_index (int): Index of jump table entry to activate after firing + this one. Note: jump_table gets an extra NOP start entry at run time + in self.toString, so first user entry is index 1. counter (int): Which counter to increment at each cycle. """ NAME = "CYCLE" @@ -262,22 +268,26 @@ class JumpTable(object): TODO: make self.jumps a property with some checking on type and number. """ - PACKET_LEN = 528 + + # TODO: Move these hardcoded FPGA values COUNTER_BITS = 32 # 32 bit register for counters COUNT_MAX = 2**COUNTER_BITS - 1 NUM_COUNTERS = 4 - def __init__(self, start_addr=None, jumps=None, counters=None): + def __init__(self, start_addr=None, jumps=None, counters=None, + packet_len=528): """ :param start_addr: start address :param list[JumpEntry] jumps: the jump table entries :param list[int] counters: the counter values + :param int packet_len: Number of byes in jump table write packet """ self._startAddr = 0 self.counters = self._initialize_counters(counters=counters) self.start_addr = start_addr self.jumps = jumps + self.packet_len = packet_len @classmethod def _initialize_counters(cls, counters=None): @@ -313,7 +323,7 @@ def __str__(self): def toString(self): """Serialize jump table to a byte string for the FPGA""" - data = np.zeros(self.PACKET_LEN, dtype=' measure -> second pi -> measure -> end + cls.first_pi_start = 0 + cls.first_pi_end = 32 + cls.second_pi_start = 32 + cls.second_pi_end = 64 + cls.ro_start = 64 + cls.ro_end = 400 + + cls.jump_args = [ + ('JUMP', [cls.first_pi_end, cls.ro_start, 1]), + ('JUMP', [cls.ro_end, cls.second_pi_start, 2]), + ('END', [cls.ro_end]) + ] + + def make_entries(self, jump_args): + return [self.dac.make_jump_table_entry(name, args) + for name, args in jump_args] + + def test_make_jump_table_entry(self): + _ = self.make_entries(self.jump_args) + # we made it this far and we're feeling pretty good about it + + def test_make_jump_table(self): + entries = self.make_entries(self.jump_args) + self.dac.make_jump_table(entries) + # we made it this far and we're feeling pretty good about it + + def test_catch_nop_nop_too_close(self): + entries = self.make_entries([ + ('NOP', [16]), + ('NOP', [20]), + ('END', [400]) + ]) + with pytest.raises(ValueError): + self.dac.make_jump_table(entries) + + def test_catch_jump_iterated_nop_too_close(self): + + entries = self.make_entries([ + ('JUMP', [32, 16, 1]), + ('NOP', [20]), + ('END', [400]) + ]) + with pytest.raises(ValueError): + self.dac.make_jump_table(entries) + + def test_catch_jump_back_too_close(self): + + entries = self.make_entries([ + ('NOP', [32]), + ('JUMP', [100, 28, 0]), + ('END', [400]) + ]) + with pytest.raises(ValueError): + self.dac.make_jump_table(entries) + + def test_catch_idle_end_too_close(self): + + # from addr of 32 is followed by from addr of 36 (even though IDLE?) + entries = self.make_entries([ + ('IDLE', [32, 200]), + ('END', [36]) + ]) + with pytest.raises(ValueError): + self.dac.make_jump_table(entries) + + def test_nop_nop_1_clk_too_close(self): + """Test a table that executes commands 1 clock cycle too frequently""" + + from_addr_ns = self.dac.JT_MIN_FROM_ADDR * 4 + entries = self.make_entries([ + ('NOP', [from_addr_ns]), + ('NOP', [from_addr_ns + + (self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES - 1) * 4]), + ('END', [400]) + ]) + + with pytest.raises(ValueError): + self.dac.make_jump_table(entries) + + def test_nop_nop_min_clk_ok(self): + from_addr_ns = self.dac.JT_MIN_FROM_ADDR * 4 + _ = self.make_entries([ + ('NOP', [from_addr_ns]), + ('NOP', [from_addr_ns + + self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES * 4]), + ('END', [400]) + ]) + + def test_max_entries(self): + entry_sep = self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES * 4 * 2 + from_addr = (entry_sep * np.arange(self.dac.JUMP_TABLE_COUNT - 2) + + entry_sep) + entries = self.make_entries( + [('NOP', [k]) for k in from_addr] + + [('END', [from_addr[-1] + entry_sep])] + ) + self.dac.make_jump_table(entries) + # we made it this far and we're feeling pretty good about it + + def test_too_many_entries(self): + entry_sep = self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES * 4 * 2 + from_addr = (entry_sep * np.arange(self.dac.JUMP_TABLE_COUNT - 1) + + entry_sep) + entries = self.make_entries( + [('NOP', [k]) for k in from_addr] + + [('END', [from_addr[-1] + entry_sep])] + ) + with pytest.raises(ValueError): + self.dac.make_jump_table(entries) + +if __name__ == '__main__': + pytest.main(['-v', __file__])