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.

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

[RFC] Better support for magma in hwtypes #102

Open
cdonovick opened this issue Nov 15, 2019 · 2 comments
Open

[RFC] Better support for magma in hwtypes #102

cdonovick opened this issue Nov 15, 2019 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@cdonovick
Copy link
Collaborator

As I have been working on trying to get Sum types to work in magma I have realized that I have developed things in a way that is fairly incompatible with magma. There are two major issues, qualification of types (In, Out, etc...) and wiring.

@cdonovick cdonovick added the enhancement New feature or request label Nov 15, 2019
@cdonovick
Copy link
Collaborator Author

cdonovick commented Nov 15, 2019

I think qualifiers could be made using modifiers. See:
https://github.com/leonardt/hwtypes/blob/master/tests/test_modifiers.py
https://github.com/leonardt/hwtypes/blob/master/hwtypes/modifiers.py

However raw modifiers are probably not sufficient as magma does not allow operators on input types (those for which is_input() returns True) . I think this could be resolved by having the In modifier remove all methods except __init__, __new__, __del__, __getitem__, __getattribute__, __getattr__, "internal" methods (those with names _name or _name_ or __name), and class/static methods. Alternatively we could also have a whitelisting mechanism to preserve specified methods which could be defaulted to __init__, ect...

e.g.:

class Foo(hwtype):
    _in_methods_ = 'bar',
    _out_methods_ = 'baz',
    def bar(self):
        print("I'm an input or undirected")
    def baz(self):
        print("I'm an output or undirected")

foo = Foo()
foo.bar() # works
foo.baz() # works

in_foo = In(Foo)()
in_foo.bar() # works
in_foo.baz() # error

out_foo = Out(Foo)()
out_foo.bar() # error
out_foo.baz() # works

We could also build first class support for qualifiers which would make some sense. This would allow us to block certain undesirable statements like x = In(BitVector[8])(0xFF) (constants are inherently output types). Although this could be achieved in the above pattern.

class BitVector:
    _out_methods_ = 'make_constant',
    def __init__(self, value):
        ...
        self.make_constant(value) 
        # raises an error if BitVector is an input type

Perhaps the biggest reason for using a modifier as opposed to having a qualify method is that modifiers automatically build a bunch of type relationships, although this logic could be extracted so that it would be easy to use. For example, in magma it is currently illegal to say In(Bits)[8] and the following is false issubclass(In(Bits[8]), Bits[8])*. With modifiers these would work out of the box.

* The relationship between In(Bits[8]) and Bits[8] is reasonable as the operators allowed on In(Bits[8]) are a subset of those allowed on Bits[8] hence In(Bits[8]) is not a subtype of Bits[8]. However, In(Bits)[8] should be possible.

@cdonovick
Copy link
Collaborator Author

cdonovick commented Nov 15, 2019

As for wiring I think it would be pretty easy to bring in. Its a well defined operation (unification). My only concern is the use of <= as a wiring operator. First it makes static analysis / AST transformations harder because I have to reason about <= differently depending on whether I am looking at Expr(Compare(lhs, LtE, rhs)) or Compare(lhs, LtE, rhs) in some other context.
e.g.:

@rewrite
def bar(x, y):
    if x <= 0: #If(Compare(...), ...)
        y <= 0 #Expr(Compare(...)) Expr "casts" expr to stmt
    else:
        y <= 1

Secondly it forces types to reason about wiring rules / qualification which I would prefer to keep as isolated as possible. I suppose if we go the modifier approach we could insert <= into In types (while it is stripping other methods ) where def __le__(self, rhs): return self.wire(rhs). Finally it has unintuitive precedence x <= y == 0 is (x <= y) == 0 not x <= (y == 0). Now I am not opposed to a wiring operator I am just opposed to using <=.

I would propose we use @= as mentioned in phanrahan/magma#495. If we make the change I would be happy to write a refactoring tool which automatically fix any legacy magma.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants