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

Support splitting up struct method parameters into multiple input ports #729

Open
wants to merge 44 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
49532f4
Initial, hacky work on computing wrapper interface method types using…
krame505 Aug 1, 2024
363cc7b
Attempt at attachting port names with a primative on every input. Do…
krame505 Aug 8, 2024
b24ca9c
Pass the input port names by tagging methods with a new primative
krame505 Aug 10, 2024
92fa2e8
Refactor WrapMethod type class
krame505 Aug 10, 2024
c6b22fa
Input port splitting works end-to-end, modulo sanity checks and savin…
krame505 Aug 13, 2024
57258ec
Handle prefix for input port names via wrap typeclasses
krame505 Aug 13, 2024
8fdb09d
Saving port types using WrapField type class method
krame505 Aug 13, 2024
5945a7c
Bug fixes
krame505 Aug 13, 2024
25695a4
Use WrapField to determine noinline foreign function types
krame505 Aug 14, 2024
6568cc8
Cleanup, add DeepSplitPorts type class
krame505 Aug 14, 2024
1b1f033
Add primMethod wrapper prim calls in vMkRWire1
krame505 Aug 15, 2024
730f044
Update expected test output
krame505 Aug 15, 2024
7975da7
Re-add module arg port type saving, still need to do port name confli…
krame505 Aug 15, 2024
3ba06d4
Fix saving Inout port types
krame505 Aug 15, 2024
96c35e5
Update test expected output
krame505 Aug 15, 2024
d08c97a
Update expected test output
krame505 Aug 15, 2024
b651d36
Fix prefix computation in genwrap 'to' function and port saving state…
krame505 Aug 16, 2024
4020ade
Fix inadvertantly disabled type check for foreign functions
krame505 Aug 16, 2024
2c9e86e
Update test expected results
krame505 Aug 16, 2024
0c15378
Add interface port name sanity checking after elaboration
krame505 Aug 17, 2024
1ecce93
Fix bug introduced in computing split vector interface prefixes
krame505 Aug 17, 2024
936d140
Add sketch of splitting tuples
krame505 Aug 17, 2024
eccaf03
Check for clash with default clock/reset ports
krame505 Aug 17, 2024
4e48c17
Better error message for synthesizing an interface with a non-Bits me…
krame505 Aug 17, 2024
14fb02c
Cleanup trailing whitespace
krame505 Aug 17, 2024
7aea56f
Update expected results, testsuite passing
krame505 Aug 17, 2024
af1e1eb
Reorganize port splitting utilites into a seperate library, add Shall…
krame505 Aug 17, 2024
a52f2e6
More efficient implementation of splitting vectors
krame505 Aug 19, 2024
36a4dc2
Avoid extra _1 suffix for DeepSplitPorts on Int/UInt
krame505 Aug 19, 2024
8999a65
Add test cases for port splitting
krame505 Aug 19, 2024
be3d910
Add test of DeepSplitPorts with an explicit non-recursive instance
krame505 Aug 20, 2024
150d29c
Fix NoSplit instance
krame505 Aug 20, 2024
c8114b1
Add a comment
krame505 Aug 20, 2024
0ea992e
Stuff the field name in the WrapField type class as a type parameter …
krame505 Aug 20, 2024
9d7921f
Fix trailing whitespace
krame505 Aug 20, 2024
f0d8ac7
Addressing Ravi's comments
krame505 Aug 20, 2024
7067fbe
Fix more comments
krame505 Aug 21, 2024
0e23ad0
Fix testsuite failure after error message tweak
krame505 Aug 22, 2024
1329534
Record the full field name path in WrapField context, for better erro…
krame505 Aug 22, 2024
6f8c504
Don't surface implicit conditions from inside ICMethod
krame505 Aug 22, 2024
b1165c9
FIx trailing whitespace
krame505 Aug 23, 2024
0868d79
Slightly less gross list deconstruction in IExpand
krame505 Aug 23, 2024
df0f863
Add more tests
krame505 Aug 23, 2024
d7b1859
Merge remote-tracking branch 'upstream/main' into genwrap
krame505 Aug 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 275 additions & 2 deletions src/Libraries/Base1/Prelude.bs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ package Prelude(
primCharToString,
primUIntBitsToInteger, primIntBitsToInteger,

($), (∘), id, const, constFn, flip, while, curry, uncurry, asTypeOf,
($), (∘), id, const, constFn, flip, while, curry, uncurry, Curry(..), asTypeOf,
liftM, liftM2, bindM,

(<+>), rJoin,
Expand Down Expand Up @@ -171,6 +171,7 @@ package Prelude(
Tuple6, tuple6, Has_tpl_6(..),
Tuple7, tuple7, Has_tpl_7(..),
Tuple8, tuple8, Has_tpl_8(..),
AppendTuple(..), AppendTuple'(..), TupleSize(..),

-- lists required for desugaring
List(..),
Expand Down Expand Up @@ -253,7 +254,10 @@ package Prelude(
-- Generics
Generic(..), Conc(..), ConcPrim(..), ConcPoly(..),
Meta(..), MetaData(..), StarArg(..), NumArg(..), StrArg(..), ConArg(..),
MetaConsNamed(..), MetaConsAnon(..), MetaField(..)
MetaConsNamed(..), MetaConsAnon(..), MetaField(..),

primMethod, WrapField(..), WrapMethod(..), WrapPorts(..),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that primMethod here is only exported so that it can be used in PreludeBSV for vMkRWire1 and can be removed when that's resolved.

Port(..), SplitPorts(..)
) where

infixr 0 $
Expand Down Expand Up @@ -2589,6 +2593,23 @@ curry f x y = f (x, y)
uncurry :: (a -> b -> c) -> ((a, b) -> c)
uncurry f (x, y) = f x y

-- Polymorphic, N-argument version of curry/uncurry
class Curry f g | f -> g where
curryN :: f -> g
uncurryN :: g -> f

instance (Curry (b -> c) d) => Curry ((a, b) -> c) (a -> d) where
curryN f x = curryN $ \y -> f (x, y)
uncurryN f (x, y) = uncurryN (f x) y

instance Curry (() -> a) a where
curryN f = f ()
uncurryN f _ = f

instance Curry (a -> b) (a -> b) where
curryN = id
uncurryN = id

--@ Constant function
--@ \index{const@\te{const} (Prelude function)}
--@ \begin{libverbatim}
Expand Down Expand Up @@ -3372,6 +3393,43 @@ tuple7 a b c d e f g = (a,b,c,d,e,f,g)
tuple8 :: a -> b -> c -> d -> e -> f -> g -> h -> Tuple8 a b c d e f g h
tuple8 a b c d e f g h = (a,b,c,d,e,f,g,h)

class AppendTuple a b c | a b -> c where
appendTuple :: a -> b -> c
splitTuple :: c -> (a, b)

instance AppendTuple a () a where
appendTuple x _ = x
splitTuple x = (x, ())

-- The above instance should take precedence over the other cases that assume
-- b is non-unit. To avoid overlapping instances, the below are factored out as
-- a seperate type class:
instance (AppendTuple' a b c) => AppendTuple a b c where
appendTuple = appendTuple'
splitTuple = splitTuple'

class AppendTuple' a b c | a b -> c where
appendTuple' :: a -> b -> c
splitTuple' :: c -> (a, b)

instance AppendTuple' () a a where
appendTuple' _ = id
splitTuple' x = ((), x)

instance AppendTuple' a b (a, b) where
appendTuple' a b = (a, b)
splitTuple' = id

instance (AppendTuple' a b c) => AppendTuple' (h, a) b (h, c) where
appendTuple' (x, y) z = (x, appendTuple' y z)
splitTuple' (x, y) = case splitTuple' y of
(w, z) -> ((x, w), z)

class TupleSize a n | a -> n where {}
instance TupleSize () 0 where {}
instance TupleSize a 1 where {}
instance (TupleSize b n) => TupleSize (a, b) (TAdd n 1) where {}

-- FUNCTIONS TO REPLACE UNAVAILABLE INFIXES

compose :: (b -> c) -> (a -> b) -> (a -> c)
Expand Down Expand Up @@ -4370,3 +4428,218 @@ data (MetaConsAnon :: $ -> # -> # -> *) name idx nfields = MetaConsAnon
-- field) and index in the constructor's fields
data (MetaField :: $ -> # -> *) name idx = MetaField
deriving (FShow)


-- Tag a method with metadata.
-- Currently just the list of input port names.
-- Should eventually include the output port names, when we support multiple output ports.
primitive primMethod :: List String -> a -> a

-- Convert bewtween a field in an interface that is being synthesized,
-- and a field in the corresponding field in the generated wrapper interface.
-- Also takes the name of the field for error reporting purposes.
class (WrapField :: $ -> * -> * -> *) name f w | name f -> w where
-- Given a proxy value for the field name, and the values of the prefix and arg_names pragmas,
-- converts a synthesized interface field value to its wrapper interface field.
toWrapField :: StrArg name -> String -> List String -> f -> w

-- Given a proxy value for the field name, converts a wrapper interface field value
-- to its synthesized interface field.
fromWrapField :: StrArg name -> w -> f

-- Save the port types for a field in the wrapped interface, given the module name
-- and the prefix, arg_names and result pragmas.
saveFieldPortTypes :: StrArg name -> f -> Maybe Name__ -> String -> List String -> String -> Module ()

instance (WrapMethod m w) => (WrapField name m w) where
toWrapField _ prefix names =
let baseNames = methodArgBaseNames (_ :: m) prefix names 1
in primMethod (inputPortNames (_ :: m) baseNames) ∘ toWrapMethod
fromWrapField _ = fromWrapMethod
saveFieldPortTypes _ _ modName prefix names =
let baseNames = methodArgBaseNames (_ :: m) prefix names 1
in saveMethodPortTypes (_ :: m) modName baseNames

-- TODO: It doesn't seem possible to have a PrimAction field in a synthesized interface,
-- but this case was being handled in GenWrap.
instance WrapField name PrimAction PrimAction where
toWrapField _ _ _ = id
fromWrapField _ = id
saveFieldPortTypes _ _ _ _ _ _ = return ()

instance WrapField name Clock Clock where
toWrapField _ _ _ = id
fromWrapField _ = id
saveFieldPortTypes _ _ _ _ _ _ = return ()

instance WrapField name Reset Reset where
toWrapField _ _ _ = id
fromWrapField _ = id
saveFieldPortTypes _ _ _ _ _ _ = return ()

instance (Bits a n) => WrapField name (Inout a) (Inout_ n) where
toWrapField _ _ _ = primInoutCast0
fromWrapField _ = primInoutUncast0
saveFieldPortTypes _ _ modName _ _ result = primSavePortType modName result $ typeOf (_ :: (Inout a))

class WrapMethod m w | m -> w where
-- Convert a synthesized interface method to its wrapper interface method.
toWrapMethod :: m -> w

-- Convert a wrapper interface method to its synthesized interface method.
fromWrapMethod :: w -> m

-- Compute the actual argument base names for a method, given the prefix and arg_names pragmas.
methodArgBaseNames :: m -> String -> List String -> Integer -> List String

-- Compute the list of input port names for a method, from the argument base names.
inputPortNames :: m -> List String -> List String

-- Save the port types for a method, given the module name, argument base names and result name.
saveMethodPortTypes :: m -> Maybe Name__ -> List String -> String -> Module ()

instance (SplitPorts a p, TupleSize p n, WrapPorts p pb, WrapMethod b v, Curry (pb -> v) w) =>
WrapMethod (a -> b) w where
toWrapMethod f = curryN $ toWrapMethod ∘ f ∘ unsplitPorts ∘ unpackPorts
fromWrapMethod f = fromWrapMethod ∘ uncurryN f ∘ packPorts ∘ splitPorts

methodArgBaseNames _ prefix (Cons h t) i = Cons
-- arg_names can start with a digit
(if prefix == "" && not (isDigit $ stringHead h) then h else prefix +++ "_" +++ h)
krame505 marked this conversation as resolved.
Show resolved Hide resolved
(methodArgBaseNames (_ :: b) prefix t $ i + 1)
methodArgBaseNames _ prefix Nil i = Cons
(prefix +++ "_" +++ integerToString i)
(methodArgBaseNames (_ :: b) prefix Nil $ i + 1)

inputPortNames _ (Cons h t) = checkPortNames (_ :: a) h `listPrimAppend` inputPortNames (_ :: b) t
inputPortNames _ Nil = error "inputPortNames: empty arg names list"

saveMethodPortTypes _ modName (Cons h t) result = do
savePortTypes (_ :: p) modName $ checkPortNames (_ :: a) h
saveMethodPortTypes (_ :: b) modName t result
saveMethodPortTypes _ _ Nil _ = error "saveMethodPortTypes: empty arg names list"

instance (Bits a n) => WrapMethod (ActionValue a) (ActionValue_ n) where
toWrapMethod = toActionValue_
fromWrapMethod = fromActionValue_
methodArgBaseNames _ _ _ _ = Nil
inputPortNames _ _ = Nil
saveMethodPortTypes _ modName _ result = primSavePortType modName result $ typeOf (_ :: a)

instance (Bits a n) => WrapMethod a (Bit n) where
toWrapMethod = pack
fromWrapMethod = unpack
methodArgBaseNames _ _ _ _ = Nil
inputPortNames _ _ = Nil
saveMethodPortTypes _ modName _ result = primSavePortType modName result $ typeOf (_ :: a)

{-
Eventually, we should support splitting multiple output ports.
instance (SplitPorts a p, TupleSize p n, WrapPorts p pb) => WrapMethod (ActionValue a) (ActionValue pb) where
toWrapMethod = fmap packPorts
fromWrapMethod = fmap unpackPorts
outputPortNames _ base = checkPortNames (_ :: a) base
saveMethodPortTypes _ modName _ result =
savePortTypes (_ :: p) modName $ checkPortNames (_ :: a) result

instance (SplitPorts a p, TupleSize p n, WrapPorts p pb) => WrapMethod a pb where
toWrapMethod a = packPorts a
fromWrapMethod a = unpackPorts a
outputPortNames _ base = checkPortNames (_ :: a) base
saveMethodPortTypes _ modName _ result =
savePortTypes (_ :: p) modName $ checkPortNames (_ :: a) result
-}

class WrapPorts p pb | p -> pb where
-- Convert from a tuple of values to a tuple of bits.
packPorts :: p -> pb
-- Convert from a tuple of bits to a tuple of values.
unpackPorts :: pb -> p
-- Save the port types, given their names.
savePortTypes :: p -> Maybe Name__ -> List String -> Module ()

instance (Bits a n, WrapPorts b bb) => WrapPorts (Port a, b) (Bit n, bb) where
packPorts (Port a, b) = (pack a, packPorts b)
unpackPorts (a, b) = (Port $ unpack a, unpackPorts b)
savePortTypes _ modName (Cons h t) = do
primSavePortType modName h $ typeOf (_ :: a)
savePortTypes (_ :: b) modName t
savePortTypes _ _ Nil = error "savePortTypes: empty port names list"

instance (Bits a n) => WrapPorts (Port a) (Bit n) where
packPorts (Port a) = pack a
unpackPorts = Port ∘ unpack
savePortTypes _ modName (Cons h _) = primSavePortType modName h $ typeOf (_ :: a)
savePortTypes _ _ Nil = error "savePortTypes: empty port names list"

instance WrapPorts () () where
packPorts _ = ()
unpackPorts _ = ()
savePortTypes _ _ _ = return ()

-- Compute the list port names for type 'a' given a base name.
-- Check that the number of port names matches the number of ports.
-- This error should only occur if there is an error in a WrapPorts instance.
checkPortNames :: (SplitPorts a p, TupleSize p n) => a -> String -> List String
checkPortNames proxy base =
let pn = portNames proxy base
in
if listLength pn /= valueOf n
then error $ "SplitPorts: " +++ base +++ " has " +++ integerToString (valueOf n) +++
" ports, but " +++ integerToString (listLength pn) +++ " port names were given"
else pn

class SplitPorts a p | a -> p where
-- Convert a value to a tuple of values corresponding to ports.
splitPorts :: a -> p
-- Combine a tuple of values corresponding to ports into a value.
unsplitPorts :: p -> a
-- Compute the list of port names for a type, given a base name.
-- This must be the same length as the tuple of values.
-- XXX it would be nice to use ListN here to enforce this, but it's not
-- available in the Prelude.
portNames :: a -> String -> List String

data Port a = Port a
deriving (FShow)

-- XXX if the default instance is the only one, then it gets inlined in CtxReduce
-- and other instances for this class are ignored.
instance SplitPorts () () where
splitPorts = id
unsplitPorts = id
portNames _ _ = Nil

-- Default instance: don't split anything we don't know how to split.
instance SplitPorts a (Port a) where
splitPorts = Port
unsplitPorts (Port a) = a
portNames _ base = Cons base Nil

{-
XXX Consider if we want to split tuples by default. This would change the current behavior,
but might be a sensible one, especially if we support methods with multiple output ports.

instance (SplitTuplePorts (a, b) r) => SplitPorts (a, b) r where
splitPorts = splitTuplePorts
unsplitPorts = unsplitTuplePorts
portNames = splitTuplePortNames 1

class SplitTuplePorts a p | a -> p where
splitTuplePorts :: a -> p
unsplitTuplePorts :: p -> a
splitTuplePortNames :: Integer -> a -> String -> List String

instance (SplitPorts a p, SplitTuplePorts b q, AppendTuple p q r) => SplitTuplePorts (a, b) r where
splitTuplePorts (a, b) = splitPorts a `appendTuple` splitTuplePorts b
unsplitTuplePorts x = case splitTuple x of
(a, b) -> (unsplitPorts a, unsplitTuplePorts b)
splitTuplePortNames i _ base =
portNames (_ :: a) (base +++ "_" +++ integerToString i) `listPrimAppend`
splitTuplePortNames (i + 1) (_ :: b) base

instance (SplitPorts a p) => SplitTuplePorts a p where
splitTuplePorts = splitPorts
unsplitTuplePorts x = unsplitPorts x
splitTuplePortNames i _ base = portNames (_ :: a) $ base +++ "_" +++ integerToString i
-}
12 changes: 7 additions & 5 deletions src/Libraries/Base1/PreludeBSV.bsv
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,17 @@ interface VRWireN#(numeric type n);
endinterface

// for addCFWire desugaring
// This uses prim types like something coming from genwrap.
module vMkRWire1(VRWireN#(1));

(* hide *)
VRWire#(Bit#(1)) _rw <- vMkRWire;
method wset(v);
return(toPrimAction(_rw.wset(v)));
endmethod
method wget = _rw.wget;
method whas = pack(_rw.whas);
function rw_wset(v);
return toPrimAction(_rw.wset(v));
endfunction
method wset = primMethod(Cons("v", Nil), rw_wset);
method wget = primMethod(Nil, _rw.wget);
method whas = primMethod(Nil, pack(_rw.whas));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user writes a conflict_free attribute for two rules, then BSC will add logic to dynamically check that any conflicting methods between the two have not been called at the same time. BSC does this by instantiating an RWire for each method called by each rule, and inserting a wset action at the place in the rule where it calls the method; then a $display is inserted into the module, which prints an error message if the wires for two conflicting methods is ever true.

This occurs in BSC after scheduling, when the module is in ASyntax. To add a submodule instantiation at that point, BSC needs an AVInst value. The reason that vMkRWire exists is so that BSC can get its hands on the AVInst for RWire. It does this by simulating the entire compilation flow for vMkRWire -- first typecheck, then convert to ISyntax, then elaboration, then convert to ASyntax -- and then looking for the sole submodule in the resulting ASyntax.

The interface for vMkRWire is entirely unneeded -- BSC just wants to get the AVInst structure for the _rw submodule instantiation. We might consider changing this module to have an empty interface, and then you don't need to worry about adding primMethod to any methods.

The reason that you needed to add primMethod is because BSC's simulated compilation flow starts at typecheck, without any GenWrap stage. It used to be OK for BSC to skip GenWrap, because later stages only required that the interface be raw types (bit and PrimAction). But you've changed the evaluator to expect that all methods evaluate to IMethod, and that only happens when there's a call to primMethod, and that gets inserted by the toWrapField call that GenWrap now inserts on the interface. So AAddSchedAssumps would need to simulate the GenWrap step by inserting toWrapField; but if that's not done, then you would need to manually insert the toWrapField, or manually write the result of that (by inserting primMethod on each method).

I think it might be better to write toWrapField on _rw, rather than explicitly spell out the interface; and better than that would be to update AAddSchedAssump to insert it; but better still would be to eliminate the interface on vMkRWire1 altogether -- but actually, I would prefer that we eliminate the simulated compile flow altogether and just hardcode an AVInst value for RWire in AAddSchedAssump.

However, all of this does make me wonder: For imported modules in BH code, we have written them as an import of the raw interface and then a wrapper module (that converts from the raw interface and inserts calls to primitives to save types etc) -- for example, mkReg is a wrapper around vMkReg. Can we simplify those wrappers now with just a call to toWrapField?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it looks like someone added a vMkUnsafeRWire1 version of this module, when they were creating Unsafe versions of all the RWire modules, but there's no need for that, so I'd delete that module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I haven't dug into AAddSchedAssump suffieciently to understand what is going on there. Although constructing an AVInst directly does seem a lot nicer than simulating the whole compilation flow, if doing so is reasonable.

I did try changing the primMethod to toWrapField, but that doesn't work because VRWireN.wset returns a PrimAction, but toWrapField uses ActionValue_ 0. The implementation of ActionValue_ is not exported from the Prelude, and even if it was, we can't manipulate the result of toWrapField since it is wrapped in primMethod. I suppose we could make toWrapMethod use PrimAction instead of ActionValue_ 0 for wrapping ActionValues? But this is likely to change again anyway in the future when we add support for methods with multiple output ports.

I suppose it is possible to use toWrapMethod in the manually-written wrapper modules for imported Verilog. Although in those cases the wrapped interface field types need to be written down anyway. Also this would require fixing the above-mentioned PrimAction/ActionValue_ 0 disparity.


endmodule

Expand Down
Loading
Loading