-
Notifications
You must be signed in to change notification settings - Fork 6
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
replace toml to yaml #276
base: master
Are you sure you want to change the base?
replace toml to yaml #276
Conversation
examples/microarch.yml
Outdated
networks: | ||
- name: net1 | ||
pus: | ||
- type: SPI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use name as key here? Do you see any disadvantages of this approuch?
The same for the network
isSlave: true | ||
bufferSize: 6 | ||
bounceFilter: 0 | ||
protos: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let move it to the top level as puLibrary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is is about all PUs, which nitta can allocate automatically. Current -- in network. Should be -- on the top level & network.
examples/microarch.yml
Outdated
name: fram{x} # If you want a PU can be allocated only once, remove {x} from the PU name. | ||
size: 32 | ||
- type: Shift | ||
name: shift{x} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what this special syntax
means?
examples/microarch.yml
Outdated
isInt: true | ||
- type: Divider | ||
name: div{x} | ||
mock: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be nice to make possible to set this attr globally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mock -- attr used for logic simulation via icarus verilog for complex PU, like divider.
Usually, we don't need to specify it for PUs independently, so it will be nice to move it on the top level.
newtype MicroarchitectureConf = MicroarchitectureConf | ||
{ networks :: [NetworkConf] | ||
data MicroarchitectureConf = MicroarchitectureConf | ||
{ type' :: T.Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to name it like valueType
. '
can be confusing here.
In the future, it is possible to make type
specific for each network.
{ networks :: [NetworkConf] | ||
data MicroarchitectureConf = MicroarchitectureConf | ||
{ type' :: T.Text | ||
, ioSync' :: IOSynchronization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same idea on '
Nothing -> return Nothing | ||
Just path -> Just . getToml <$> T.readFile path | ||
Just path -> Just <$> parseConfig path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like parseConfig <$> path
should work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parseConfig
is a FilePath -> IO MicroarchitectureConf
type
path
is a FilePath
type
<$>
is a (a -> b) -> f a -> f b
type
The result must be type of IO Maybe MicroarchitectureConf
, so Just <$> parseConfig path
doesn't seem to be able to be simplified.
} | ||
deriving (Generic, Show) | ||
|
||
instance FromJSON MicroarchitectureConf | ||
instance FromJSON MicroarchitectureConf where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to do it via Generic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't work out ¯\_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange. Maybe because you need to use HashMap instead of Map.
type' <- v .: "type" | ||
ioSync' <- v .: "ioSync" | ||
networks <- v .: "networks" | ||
return MicroarchitectureConf{type' = type', ioSync' = ioSync', networks = networks} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decodeFileThrow path :: IO MicroarchitectureConf | ||
|
||
mkMicroarchitecture :: (Val v, Var x, ToJSON x) => MicroarchitectureConf -> BusNetwork T.Text x v Int | ||
mkMicroarchitecture conf = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use record syntax here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better. Can you add generation of configuration file to the target project and small unit test, like it described in #263
After that I will ry it locally.
examples/microarch.yml
Outdated
mock: true | ||
type: fx32.32 | ||
ioSync: Sync | ||
puLibrary: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Here we have misunderstanding. The idea behind this: describe library of process unit to be allocated in separated place, with name "library" intead of "proto"
} | ||
deriving (Generic, Show) | ||
|
||
instance FromJSON MicroarchitectureConf | ||
instance FromJSON MicroarchitectureConf where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange. Maybe because you need to use HashMap instead of Map.
No description provided.