hwtypes icon indicating copy to clipboard operation
hwtypes copied to clipboard

[RFC] Better support for magma in hwtypes

Open cdonovick opened this issue 5 years ago • 2 comments

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 avatar Nov 15 '19 22:11 cdonovick

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 avatar Nov 15 '19 22:11 cdonovick

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 https://github.com/phanrahan/magma/issues/495. If we make the change I would be happy to write a refactoring tool which automatically fix any legacy magma.

cdonovick avatar Nov 15 '19 22:11 cdonovick