<sb0>
larsc: you may have problems with 1) the idiosyncratic out-of-band signaling of SATA 2) recovering RX clock unless the hard disk does loop timing (I don't know if it does)
[X-Scale] has joined #m-labs
X-Scale has quit [Ping timeout: 264 seconds]
[X-Scale] is now known as X-Scale
futarisIRCcloud has quit [Quit: Connection closed for inactivity]
<GitHub-m-labs>
[artiq] whitequark commented on issue #1007: Yes, effectively turning Python classes into Rust traits, which are a much better runtime model. That would also give us a sane way to implement polymorphism. https://github.com/m-labs/artiq/issues/1007#issuecomment-435750643
<sb0>
new migen with common problems hopefully addressed and without backward compatibility (other than being installable along the current migen with legacy cores that can be instantiated)
<whitequark>
that sounds great
rohitksingh_work has joined #m-labs
<whitequark>
if you can get basic CDC, FIFOs, FSMs and Records working it can probably be tested on Glasgow as that doesn't use anything else from migen (apart from mibuild I guess), and is a reasonably nontrivial design
<sb0>
e.g. support for multiple output modules, deterministic signal names, PEP8-friendly syntax, no "self.submodules += ..." etc. that are easily forgotten
<whitequark>
I would be fine with just a __del__ on a Module that complains if it isn't plugged anywhere
<whitequark>
asyncio style
<sb0>
not just on modules, you can also forget self.comb, self.sync, etc.
<whitequark>
hm, overloading <=
<whitequark>
that's kind of gross. how do you do comparisons?
<whitequark>
can i suggest using @= instead?
<whitequark>
maybe not as verilog-y, but it is very unambiguous
<sb0>
yeah, it's a hack. if the result of "<=" isn't used in another operation, then it turns into an assignment
<whitequark>
the `with If(count == 4):` stuff seems much less elegant than current migen, but I suppose that's the cost of doing a proper eDSL
<whitequark>
i feel like nmigen should avoid such hacks altogether rather than switching to slightly different ones...
<whitequark>
imagine writing higher order code that manipulates results of comparison
<whitequark>
that is very error prone
<whitequark>
e.g. def f(x): pass; f(a <= b)
<sb0>
it's either hacks like this, or having self.comb/self.sync
<sb0>
well it doesn't have to be <=
<whitequark>
if you do it with @= you don't need a hack
<whitequark>
because it doesn't overlap with anything else on integers
<whitequark>
regarding self.comb/self.sync, the main problem with them as I see is that you have to go for NextValue/eq in FSM instead of uniformy using eq
<sb0>
okay. you still need to stack/global state though
<whitequark>
right
<whitequark>
but you can't avoid the global implicit state and have uniform syntax in FSMs and non FSMs right?
<whitequark>
at least, I do not know of any way that wouldn't be even worse
<whitequark>
whereas @= seems like a simple fix to me
<whitequark>
sb0: actually, hang on a sec
<whitequark>
I think I know a way that doesn't involve implict global state
<whitequark>
there's... somewhat more visual clutter? but this is easily offset by,
<whitequark>
a) never hunting that stray paren again (this is the #1 time sink when bringing up a new migen design i think)
<whitequark>
b) uniform comb/sync
<sb0>
what about multiple clock domains? same as now i.e. f.sync.domain?
<whitequark>
yes
<whitequark>
sb0: oh, I realized one way I don't like about your with Comb() design in particular
<whitequark>
not only it privileges one kind of logic (which is weird on its own) but it also adds another bit of global state
<whitequark>
imagine copy pasting some code from with Comb() block into a with Sync()
<whitequark>
this is *very* bad imo
<whitequark>
this is definitely going to result in hard to diagnose bugs
<sb0>
comb by default sounds fine to me, and fsm.act() already does that
<whitequark>
I've definitely hit bugs where I copied code from a sync block into fsm.act and it didn't work
<sb0>
so you want f.comb or f.sync on every single statement?
<whitequark>
blanket requiring this in nmigen is not something I would do, but I am strongly considering writing my code like that
<whitequark>
(and adding an editor extension to highlight comb/sync in different ways)
<whitequark>
sb0: there's another consideration
<whitequark>
let's say you're copying a part of f.sync += [ ... ] block
<whitequark>
when you are pasting it, you have to stop and think about context
<whitequark>
like, do you add it to an exising f.??? block? do you write a new one?
<whitequark>
because that part of block is not valid on its own
<whitequark>
if you are copying a part of nmigen code with syntax just like "count <= 0" which IS valid on its own
<whitequark>
you do not have this "speed bump"
<whitequark>
so it's easier to make this mistake than even code with multiple statement f.??? += [ ... ] blocks
<whitequark>
I think only like 30% of eDSL ergonomics is about making it readable, and the rest is about ensuring it does not break horribly when you are refactoring something
<whitequark>
by the way, I would very much like code coverage in migen or nmigen
<whitequark>
but that's orthogonal
<cr1901_modern>
"after" is definitely more difficult to read for me, but I have no idea if that's only because I'm not used to it yet. It does take my eyes longer to see If/Else conditionals
<cr1901_modern>
But it looks like it maps nicely to old migen
<whitequark>
cr1901_modern: it is more cluttered, yes
<whitequark>
but it is, in my opinion, much safer to *modify*
<whitequark>
because the clutter carries important context
<sb0>
cr1901_modern: which one? with or without f.?
<cr1901_modern>
with f.
<whitequark>
well, there is a middle ground, I guess
<whitequark>
have comb/sync but not f.
<whitequark>
but it seems like not gaining very much
<cr1901_modern>
I know it's odd, but those "f." make it more difficult for my eyes to immediately see "oh that's a conditional"
<cr1901_modern>
whitequark: I understand why it should be "f."
<whitequark>
yes, I think the "f." in "with" statements are kind of bad
<whitequark>
but I don't see a way around it that would not just produce more obscure bugs elsewhere
<cr1901_modern>
One thing in old migen is that If() returns an object where Else() can be called. In this new syntax, it looks like you can start an "If", and then create a new context that's not an Else or Elif
<cr1901_modern>
how would that be handled?
<whitequark>
the parent context of an "If" has a small FSM that handles conditionals
<whitequark>
so it knows what to do if it sees an Elif or Else
<cr1901_modern>
That'll work. Still should probably yell at someone who tries to be cute and not pair their If/Elses :)
rohitksingh_work has quit [Read error: Connection reset by peer]
<whitequark>
I mean, it'd go back to the initial state if it sees anything but Elif or Else
* cr1901_modern
thinks
<cr1901_modern>
well that's no different from verilog other than begin/end is implicit... I _think_
<cr1901_modern>
sb0: How do you do submodules w/ this new syntax?
<sb0>
cr1901_modern: nest two with Module
<cr1901_modern>
can I still create multiple "with Module()"s at file scope? Can I subclass Module so I can do "with MySubClassedModule()"?
<whitequark>
the latter is pretty much required
<cr1901_modern>
Module("top") is equivalent to "self.submodules.top += Module()" in old Migen?
<whitequark>
you need some way to do interfaces
<sb0>
you can have user classes that define the interface, and have a implement() method that creates the Module
<sb0>
then warn in __del__ if implement() is never called
<cr1901_modern>
I would have to see an example
<sb0>
there should be a way to silance that warning, e.g. if you only want a CSR map, you can skip implement(), which might take a somewhat long time on a slow machine with a big design
<whitequark>
hm, makes sense
<cr1901_modern>
Right now it doesn't look like one can import a Module defined in one file into another, because everything is stored in the global_context variable (I guess there's no way around the global state)
<cr1901_modern>
>you can have user classes that define the interface, <-- oh okay, I think I get it now. Still less boilerplate than old migen.
<sb0>
also it separates the two in a clear way, unlike the current way of doing things
<GitHub-m-labs>
[artiq] jordens commented on issue #1007: Conceptually fine. **But** can we please maintain and manage the current one first? Starting a new revision of ARTIQ-Python while the current state waits for regression fixes, documentation, performance metrics, speed improvements, and block releases is too early and too risky for my taste. https://github.com/m-labs/artiq/issues/1007#issuecomment-435796191
rohitksingh_work has quit [Read error: Connection reset by peer]
<GitHub-m-labs>
[artiq] jordens commented on issue #1007: Conceptually fine. **But** can we please maintain and manage the current ARTIQ-Python first? Starting a new revision of ARTIQ-Python while the current state waits for regression fixes, documentation, performance metrics, speed improvements, and block releases is too early and too risky for my taste. https://github.com/m-labs/artiq/issues/1007#issuecomment-435796191
rohitksingh has joined #m-labs
sb0 has quit [Ping timeout: 244 seconds]
sb0 has joined #m-labs
sb0 has quit [Ping timeout: 272 seconds]
<GitHub-m-labs>
artiq/urukul-sync f0f67b0 Robert Jördens: ad9910: add init bit explanation
mumptai has quit [Read error: Connection reset by peer]
mauz555 has joined #m-labs
mumptai has joined #m-labs
mumptai has quit [Remote host closed the connection]
mauz555 has quit [Remote host closed the connection]
mauz555 has joined #m-labs
<GitHub-m-labs>
[artiq] klickverbot commented on pull request #1185 b5a5be4: I'd categorise this as (mild) code smell – comments like these add cognitive overhead to express something that is already obvious from just reading the code as plain English.... https://github.com/m-labs/artiq/pull/1185#discussion_r230948993
mauz555 has quit [Remote host closed the connection]