SystemRDL / systemrdl-compiler

SystemRDL 2.0 language compiler front-end
http://systemrdl-compiler.readthedocs.io
MIT License
236 stars 69 forks source link

interal/external properties don't propogate #11

Closed bdmagnuson closed 6 years ago

bdmagnuson commented 6 years ago

I've tried annotating elements with external and while this is accepted by the parser the following visitor gets None for all nodes. I've tried putting external (or internal) on the component def, an instance of an anon def, and on an explicit instance. All the same.

class RTLListener(RDLListener):
    def __init__(self):
        pass

    def enter_Component(self, node):
        print(node.inst.external)

As an aside the parser rejects this. Admittedly a weird thing to do, but I think it's legal.

   external reg {
       field {} f;
    } external r;
amykyta3 commented 6 years ago

Thanks for reporting! I had this on my todo list but looks like I lost track of it. Should be pretty quick to implement internal/external propagation.

I'll double-check if the second example is valid. At a glance, I'm not sure if the grammar allows it but it is worth a closer look.

bdmagnuson commented 6 years ago

No problem! Happy to test. I did check the grammar and the 'double' component_inst_type formulation is indeed not legal. I just happened to enter that by accident when I was testing permutations and thought 'huh. well why not?'

Your project looks great though. I have my own at https://github.com/bdmagnuson/hsrdl that I've been meaning to

  1. Clean up and make more idiomatic (this was my first Haskell project)
  2. Support SRDL 2.0

Stumbling upon your project saved me a lot of time! At least for my immediate needs, even if I do have to whip up an RTL backend for it, which is precisely how I encountered the external/internal issue.

At the very least it's been interesting to see how someone else interprets some of the fuzzier parts of the spec. I still think that my favorite one is:

'A SystemRDL compiler shall imply the nature of a counter as a up counter, a down counter, or an up/down counter by the properties specified for that counter field.'

amykyta3 commented 6 years ago

If you're working on your own implementation, be sure to check out my implementation notes. These are basically excerpts from my "lab notebook" that document my reasoning for some of my design choices. Some of the stuff is pretty specific to my implementation, but maybe there will be something useful for your project. I'm also maintaining a document on any RDL spec issues/ambiguities I've come across. These are less project-specific and may be useful.

If you're putting together an RTL back-end, I'd be interested in seeing what you've done! I'm slowly putting together some exporters such as HTML and IP-XACT. Eventually these components may get tied together into a coherent command-line tool.

bdmagnuson commented 6 years ago

I'd be happy to share what I've done do far but I've had chats with new employer's lawyers and they view this sort of thing to be a little too 'work adjacent' for me to work on it in the open. You'll note the lack of commits to my repo in the past six months.

That said I'm not banned from doing so. I just need to get commits approved on an individual basis so there's some administrative overhead. It's actually a bit easier to get contributions approved to someone else's project so there's that.

I can say that the approach I've taken so far is to use one fat template with a bunch of derived parameters to lift a lot of the conditional logic out of the template. Mostly for taking different paths based on if a parameter value if unset, set to anything, boolean, numeric, or a prop reference for those parameters than can actually be any of the above. I also wrote some utility functions to get me all of the hierarchical regs/fields/signals from a given node rather than just the immediate children. I was aiming to enhance this to prune out external regs when I hit the original problem I opened this issue for.

bdmagnuson commented 6 years ago

Seems pretty good on initial inspection. Thanks for taking care of this so quickly!