Origen-SDK / o2

MIT License
4 stars 0 forks source link

stub out of to_python #61

Closed info-rchitect closed 3 months ago

info-rchitect commented 4 years ago

Sorry for all of the churn, I had to re-open with latest master. Here is the underlying issue driving this PR.

info-rchitect commented 4 years ago

@ginty As we discussed in the issue, there are some API enhancements needed. I believe I may be impeded currently as I don't see a way to programmatically create a memory map within a sub block instance.

 36  ->             with parent.memory_map(memory_map_name):
 37                     for addr_block_tag in root.find_all('spirit:addressBlock'):
 38                         addr_block_name = addr_block_tag.find_next("spirit:name").text.strip()
 39                         if addr_block_name:
 40                             breakpoint()
 41                             with parent.AddressBlock(addr_block_name):
 42                                 # Instantiate within an address block
 43                                 self.__create_regs(addr_block_tag)
 44                         else:
 45                             # Instantiate at the top level
 46                             self.__create_regs(addr_block_tag)
(Pdb) dir(parent)
['__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattr__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__proxies__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_default_default_address_block', '_load_pins', '_load_regs', '_load_sub_blocks', 'add_reg', 'add_simple_reg', 'add_sub_block', 'app', 'block_path', 'is_top', 'memory_map', 'model_id', 'name', 'path', 'pins_loaded', 'regs_loaded', 'sub_blocks_loaded', 'tree', 'tree_as_str']
(Pdb) type(parent)
<class 'origen.controller.Base'>
(Pdb) parent.memory_map(memory_map_name)
*** OSError: The block 'ip1' does not have a memory map named 'RegisterMap'

I expect parent above to have a method add_memory_map, is this a correct assumption? memory_map looks be a getter, not a setter. One other item that seems odd when compared to O1 is having to use different loaders for sub blocks and everything else. In O1 the top level DUT could create anything and sub_blocks could as well. Here I am manually passing around these loaders and I think I may be doing something incorrectly. Hopefully there is a more elegant way to create these objects that I just missed when I started hacking away.

info-rchitect commented 4 years ago

@chrisnappi I also think we really do need a better example of IP-XACT at some point. I have started googling but no luck so far.

chrisnappi commented 4 years ago

@info-rchitect - yep working on it. It is kinda surprising to find so little out there! I definitely thought it was in wider use.

ginty commented 4 years ago

I expect parent above to have a method add_memory_map, is this a correct assumption?

Yeah, there is basically no API for you currently.

I didn't really want to support it initially to force users to define everything in the block layout which means it will all lazy-load. However, I can see we need it here for import tools and the like.

So yes, you can assume a friendly API is coming (or you can add it, all Python side after all). The loaders are not intended for user use, but basically the friendly API should just wrap them - same as #52

info-rchitect commented 4 years ago

I expect parent above to have a method add_memory_map, is this a correct assumption?

Yeah, there is basically no API for you currently. I didn't really want to support it initially to force users to define everything in the block layout which means it will all lazy-load. However, I can see we need it here for import tools and the like. So yes, you can assume a friendly API is coming (or you can add it, all Python side after all). The loaders are not intended for user use, but basically the friendly API should just wrap them - same as #52

@ginty so the enhanced API would be part of the Controller Base class right?

info-rchitect commented 4 years ago

@ginty Can you help with this error regarding sub block iteration?

def test_duplicate_instance_error():
        boot_falcon()
        assert list(origen.dut.sub_blocks.keys()) == ['core0', 'core1', 'core2', 'core3']
>       for sub_block in origen.dut.sub_blocks():
E       TypeError: 'Proxy' object is not callable

tests/errors_test.py:11: TypeError
ginty commented 4 years ago

Try without ()

info-rchitect commented 4 years ago

Try without ()

info-rchitect@D10-brian:/mnt/c/o2_errors/example$ poetry run pytest tests/errors_test.py
=========================================================================================================== test session starts ============================================================================================================platform linux -- Python 3.8.1, pytest-3.10.1, py-1.8.1, pluggy-0.13.1
rootdir: /mnt/c/o2_errors/example, inifile: setup.cfg
collected 1 item                                                                                                                                                                                                                           

tests/errors_test.py F                                                                                                                                                                                                               [100%]

================================================================================================================= FAILURES =================================================================================================================______________________________________________________________________________________________________ test_duplicate_instance_error _______________________________________________________________________________________________________
    def test_duplicate_instance_error():
        dut = boot_falcon()
>       for sub_block in origen.dut.sub_blocks:

tests/errors_test.py:10:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <origen.sub_blocks.Proxy object at 0x7f43ee1a8460>, key = 0

    def __getitem__(self, key):
>       return self.__dict__[key]
E       KeyError: 0

../python/origen/sub_blocks.py:12: KeyError
========================================================================================================= 1 failed in 0.36 seconds =========================================================================================================

Key is being set to what looks more like a numeric ID than the string Python sees:

 11  ->     def __getitem__(self, key):
 12             return self.__dict__[key]
(Pdb) self.__dict__
{'core0': <origen.controller.Base object at 0x7f6ac043bdc0>, 'core1': <origen.controller.Base object at 0x7f6ac043b730>, 'core2': <origen.controller.Base object at 0x7f6ac043b0a0>, 'core3': <origen.controller.Base object at 0x7f6ac043b190>}
info-rchitect commented 4 years ago

@ginty Somewhere when creating regs and bit fields I get a Rust core dump:

-> origen.app.translate(remote_file)
(Pdb)  origen.app.translate(remote_file)

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>thread '<unnamed>' panicked at 'attempt to subtract with overflow', /mnt/c/o2/compiler/rust/origen/src/core/model/registers/register.rs:148:37
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped)

Seems likely I am passing in some wrong combination of bit field meta? Though my translator worked previously. Can you take a look? I will update my branch now.

ginty commented 4 years ago

Yeah, illegal bit field values I would think, e.g. width overrunning the size of the register or something like that. I have seen it do similar before, there needs to be either error checking added at register definition time or else error handling added to where it goes awry here, but probably the former. I was hesitant to add it when I was working in that area because at the time I was nervous about register instantiation taking too long, however I think we can accommodate validation of the user input.

If you can work out what you are doing to cause it and create a test case for it that would be awesome.

ginty commented 4 years ago

It could also be a bug of course. If you add RUST_BACKTRACE=1 to your env then it will give a Rust stack dump to identify where it is going wrong.

info-rchitect commented 4 years ago

RUST_BACKTRACE=1

This did not work for me, even after starting a new shell.