<galibert[m]>
The question was what belongs to lib, and it is a thorny one
<whitequark[cis]>
I actually found a potential motivation for a strobe generator to belong to lib
<whitequark[cis]>
see, if you are driving EnableSignal of a domain, you can potentially use platform.unsafe_add_multicycle_constraint to relax the timings
<galibert[m]>
The fixed strobe generator could publish its configuration indeed
<whitequark[cis]>
this would have to be coupled with the logic that ensures that the invariant of the multicycle constraint is preserved, or adding such a constraint can potentially completely break your design
Wanda[cis] has joined #amaranth-lang
<Wanda[cis]>
meow again
<whitequark[cis]>
so FixedStrobeGenerator is in scope conditionally on those two features being added first (EnableSignal and platform.unsafe_add_multicycle_constraint)
<tpw_rules>
how is VariableStrobeGenerator with a const input different to FixedStrobeGenerator
<whitequark[cis]>
I don't see a strong motivation for adding the others. "best practices" is a matter of opinion to an extent that makes the concept almost meaningless, so that's not something I view as reasonable motivation
<galibert[m]>
tpw: gcd reduction
<galibert[m]>
Catherine: yet the crc is in
<galibert[m]>
(and I don't want to crc out)
<tpw_rules>
galibert[m]: the toolchain cannot simplify that?
<whitequark[cis]>
galibert: I don't recall "best practice" being a motivation there
<galibert[m]>
tpw_rules: not a chance
<whitequark[cis]>
the motivation there is "there is basically one reasonable way to do this"
<tpw_rules>
i guess i would have to see the implementation to understand
<whitequark[cis]>
does anyone have anything new to say about #26?
<galibert[m]>
Catherine: I'd be very interested to be taught a better way than the #900 implementation
<whitequark[cis]>
galibert: the other problem is that I don't think strobe generation is important enough to warrant adding, and teaching, three new classes which have overlapping applicability
<galibert[m]>
I'm ok with dropping the weird one
<whitequark[cis]>
it's not particularly complex to do this at the point of use
<tpw_rules>
i do think it needs to be one class, this would follow under being okay with a little more logic to remove footguns
<Chips4MakersakaS>
I'm sure it is new but don't like base_frequency and strobe_frequency as init parameters; prefer strobe every nth cycle. Assumptions base frequency is fixed may not be future proof.
<Chips4MakersakaS>
* I'm not sure...
<galibert[m]>
Chips4Makers (aka Staf Verhaegen): Fixed fixes both, Variable leaves both free. And one of the points of the implementation is to allow optimally dithered non-power-of-two ratios
<whitequark[cis]>
that's a really niche use case
<galibert[m]>
really?
<whitequark[cis]>
yeah? I've never wanted it
<galibert[m]>
like... serial on wishbone is niche?
<whitequark[cis]>
serial on wishbone needs that?
<whitequark[cis]>
(I don't think that needs an enable inserter at all)
<galibert[m]>
serial on wishbone needs some kind of frequency generator
<whitequark[cis]>
yeah, a counter
<whitequark[cis]>
basically every UART in existence uses a baud rate divisor
<zyp[m]>
fractional dividers are common in UARTs
jfng[m] has joined #amaranth-lang
<jfng[m]>
the serial core in -stdio uses a `divisor` parameter, rather than a baudrate
<zyp[m]>
which is effectively what galibert means
<whitequark[cis]>
I've never seen one, actually
<cr1901_>
msp430 has one
<whitequark[cis]>
do you have a link to a pdf?
<zyp[m]>
my impression is that most modern MCUs have fractional dividers in their UARTs, stm32 certainly do
<galibert[m]>
(serial also means i2c, obviously)
<Chips4MakersakaS>
I'm thinking more of CPU design where you do part of it on half frequency but don't fix the frequency of the core. That depends on the FPGA or ASIC technology.
<whitequark[cis]>
I stand corrected (though it still uses a divisor and not a frequency input)
<whitequark[cis]>
however, I'm not sure if this RFC is actually applicable to this use case
<whitequark[cis]>
if you use an UART, you probably don't want to use EnableInserter at all in your gateware, and you probably want oversampling too
<whitequark[cis]>
my overarching point is that reset and clock (and enable strobe) generation logic is usually a fair bit more complex than this and it has parts that are integrally tied together
<galibert[m]>
The class is generating a signal you can use in m.If, you don't have to use EnableInserter
<whitequark[cis]>
the reason I bring up EnableInserter is that having (optionally, if possible) a multicycle clock constraint applied to the design is thus far the one motivation to have this in the core language that I see
<zyp[m]>
galibert[m]: that's fine for a transmitter, but for an async receiver you still want to have resync logic on edges
<galibert[m]>
zyp: usually you want to have your clock at 16 times the transmission frequency, yes
pbsds has joined #amaranth-lang
<whitequark[cis]>
16 seems bad for tie-breaking
<galibert[m]>
there's no tie breaking, there's only phase matching...?
<whitequark[cis]>
if you oversample you want to apply a majority function to the samples
<whitequark[cis]>
oh, i guess you could apply a divider before even clocking the UART at all to not run it at core frequency of hundreds of MHz, but ... there's no real harm in doing that\
<whitequark[cis]>
* oh, i guess you could apply a divider before even clocking the UART at all to not run it at core frequency of hundreds of MHz, but ... there's no real harm in doing that
<Wanda[cis]>
power saving ig
<whitequark[cis]>
then you want to run it off some other slower clock (at that clock)
pie_ has quit [Ping timeout: 255 seconds]
<whitequark[cis]>
this is how it works in stm32s for example
<galibert[m]>
and you create the slower clock by gating the faster one
<zyp[m]>
the SWO/UART receiver I wrote for orbtrace samples at 500 MS/s, works reliably up to 60-70 Mbaud or so…
<whitequark[cis]>
galibert[m]: that really depends on the design
<whitequark[cis]>
it could be a different output of the PLL, and often is
<whitequark[cis]>
or it could be the crystal clock before the PLL, which is another common choice
<whitequark[cis]>
you also often see a fixed divider, not an enable but a proper divider, because that saves power
<Chips4MakersakaS>
Is there other use other than serial comm. for using divisor or base/strobe freq. ?
<galibert[m]>
In any case, the core of your objection is that it may be useful but it's not useful enough to warrant being in lib, because lib has become part of the language definition when you went with lib.data and lib.wiring and all the future others
<Chips4MakersakaS>
*use case
<galibert[m]>
I'm even ok with that. But we could use a place for the useful stuff
<whitequark[cis]>
galibert: I'm happy to discuss improvements in discoverability of Amaranth libraries
<tpw_rules>
i vote close for similar reasons, the API surface and discoverability are big ones
<whitequark[cis]>
ok, cr1901: close
<Wanda[cis]>
yeah, close, doesn't seem like stdlib is the right place for that kind of complexity
<Wanda[cis]>
(though I'm not super convinced of this decision)
<Chips4MakersakaS>
Also close.
<zyp[m]>
agreed, close
<jfng[m]>
abstain, i don't have recent experience with this topic
pie_ has joined #amaranth-lang
<galibert[m]>
I'll just note that under the current criteria crc, coding and fifo would not be acceptable in lib
<whitequark[cis]>
that's partially true
<whitequark[cis]>
I think coding doesn't have a place in lib
<whitequark[cis]>
it's a bad API and it's exposed for bad reasons
<whitequark[cis]>
in any case, I will discourage you from interpreting criteria as a set of hard rules; building programming languages involves a lot of judgement calls, and I'm not having these meetings so that people try to rules-lawyer them; I am having them so that we all build a better language
<jfng[m]>
i use its one-hot/binary primitives frequently actually, but i think they are way too simplistic for general usefulness
<whitequark[cis]>
if someone proposed coding today I would have voted against it and I regret adding it
<cr1901_>
galibert[m]: As much as I hate saying it, you're not going to want to implement your own async fifo. And stdlib has best discoverability
<jfng[m]>
we have dealt with similar cases (e.g. `lib.scheduler`) in the past, by removing them from stdlib
<whitequark[cis]>
my views on fifo and crc (as well as prbs that we hopefully will get) remain the same: the clear and universal usefulness and non-contentious nature makes them very good candidates for being in the stdlib
<galibert[m]>
crc has very annoying limitations though
<whitequark[cis]>
I agree
<whitequark[cis]>
I think those should be lifted, and we'll hopefully see work on that when streams are added to CRC
<whitequark[cis]>
if these cannot be lifted and they severely limit the usefulness of CRC (by reducing universality), I'm open to moving it to an external library
<jfng[m]>
should we deprecate `lib.coding` for 0.5 ?
<zyp[m]>
jfng[m]: if you write a RFC for it :)
<whitequark[cis]>
jfng: I think I don't have time to propose that but I'm happy with that proposal
<galibert[m]>
I'd suggest though to have coding/crc/fifo and data/enum/io/wiring to be in different directories. Not sure about cdc honestly. Because they're really not the same thing
<whitequark[cis]>
I can't imagine we want to go ahead continuing to have the existing API
<jfng[m]>
zyp[m]: yeah
<Wanda[cis]>
cdc is a hardware primitive, kind of.
<Wanda[cis]>
(or, on some FPGAs, not just "kind of", though we don't make use of that rn)
<whitequark[cis]>
I think the distinction of "everything that isn't a part of the compiler and doesn't live in hdl goes to lib" is a decent one
<whitequark[cis]>
Rust's libcore is Amaranth's lib
<whitequark[cis]>
so there's precedent for that
<whitequark[cis]>
actually, no, Amaranth's distinction is even stronger than Rust's
<galibert[m]>
it's very iffy that data and wiring and io are not considered part of the compiler honestly
<whitequark[cis]>
we're taking things out of the compiler recently, not adding more! Memory being the latest example
<Wanda[cis]>
(unrelatedly to current discussion, I'd also like to see some day if xilinx hard FIFO* primitives could be used as an alt backend for amaranth.lib.fifo)
<Wanda[cis]>
(funnily enough, those are where the now-gone fwft functionality and naming comes from)
<whitequark[cis]>
yeah, that would be another reason to have lib.fifo in the stdlib: integration with platforms
<galibert[m]>
Catherine: not being directly supported by the compiler does not make std::move any less part of c++ (and in fact the requirement of an include is very annoying there)
<galibert[m]>
it's more about the perimeter of the language definition
<whitequark[cis]>
maybe we should not take example from C++?
<whitequark[cis]>
I think the distinction we have is important because it lets people feel more comfortable about the question of "what if lib.data does not fit my use case?"
<whitequark[cis]>
because, indeed, there were some use cases for Record that weren't covered by lib.wiring or lib.data
<whitequark[cis]>
zyp: should I nominate #36? it's a draft
<zyp[m]>
yes, please
<zyp[m]>
by the way, do we have some sort of policy for how to use draft PRs in the RFC repo?
<whitequark[cis]>
I'd say "undraft it when it's ready for the meeting"
<galibert[m]>
errr, passive?
<zyp[m]>
when it's ready for a vote, maybe? to take #41 as an example, I'd like to discuss that in a meeting to get more feedback, but it's gonna need more work before we can vote on it
<whitequark[cis]>
sounds good
<zyp[m]>
okay, I'll undraft #36 then
Darius has joined #amaranth-lang
<galibert[m]>
zyp: any sane decision will require performance numbers fwiw. Current sim is... not very fast, if it gets any slower it's useless
<whitequark[cis]>
it's quite fast, I think
<galibert[m]>
that would be cool
<whitequark[cis]>
migen's a few times slower and litex lives with it
<galibert[m]>
not a reason to make amaranth's sim slower though :-)
<whitequark[cis]>
I think having a better API is a perfectly fine reason to make sim slower
<zyp[m]>
it's not slower, the initial RFC #36 implementation is just syntax sugar over the existing implementation
<whitequark[cis]>
we have already done this with add_testbench
<zyp[m]>
so sim.tick().until(ready) is equivalent to while not (yield ready): yield Tick() and would perform the same
<zyp[m]>
the point is that once we have the API, we can replace the implementation later and offload that entire check to CXXRTL
<galibert[m]>
Suggestions for next meeting: some numbers that show there's no significant slowdown even with the layer you added, document passive, and possibly allow sim.until(...) as a shortcut for sim.tick().until(...)
<Wanda[cis]>
no, no numbers
<Wanda[cis]>
this is not the final implementation
<Wanda[cis]>
we want to design the API
<zyp[m]>
the proposal is about the API, not the implementation
<galibert[m]>
Wanda: you're confident that a final implementation of such an api will be at least as fast as the generator one?
<Wanda[cis]>
for one, the goal is to get rid of the current yield-based API, and make this the native layer
<Wanda[cis]>
and why exactly would it not be?
<zyp[m]>
async functions are just generators with sugar
<galibert[m]>
I have no idea, you know the internal mechanics of both async and generators much better than me
<galibert[m]>
zyp: you realize that you're saying that it can only be slower than generators, only hopefully not significantly so when you say that
<zyp[m]>
sure
<Wanda[cis]>
that's a pretty strange concern in the first place, given that the simulation API consumes next to no time
<zyp[m]>
the difference between generators and async functions are in readability, any speed difference should be negligible
<Wanda[cis]>
the time consuming part is the actual circuit
<galibert[m]>
but if you of you tell me "I know the contents of said sugar, I'm certain it's not going to make a significant difference" I believe you
<Wanda[cis]>
which doesn't make use of generators at all
<Wanda[cis]>
FWIW async does not use generators undearneath
<galibert[m]>
s/you/one/
<Wanda[cis]>
it just happens to be implemented though a very similar mechanism in Python
<galibert[m]>
so they're not generators-with-sugar then?
<zyp[m]>
Wanda[cis]: huh? `await foo` is effectively equivalent to `yield from foo.__await__()`
<Wanda[cis]>
yeah, it's the same mechanism, and they're to some extent interoperable due to compatibility reasons
<Wanda[cis]>
but it's not some kind of extra layer implemented over actual generator objects
<Wanda[cis]>
like, in case you'd be worried about any kind of performance loss from the layers
<Wanda[cis]>
there just ... aren't any
<galibert[m]>
Ok, then it's good
<galibert[m]>
Like I said, you're very competent in that area, I believe you
smkz has quit [Quit: smkz]
smkz has joined #amaranth-lang
smkz has quit [Client Quit]
<tpw_rules>
Wanda[cis]: just wanted to make sure the quartus DSP issue didn't get lost, not sure how "not now" you meant"
smkz has joined #amaranth-lang
<tpw_rules>
you said you had a solution to the issue
<Wanda[cis]>
oh that, yes
<Wanda[cis]>
I say many things
<Wanda[cis]>
sometimes I even implement them
<Wanda[cis]>
I uh
<Wanda[cis]>
let me reload context
<galibert[m]>
IIRC, context is losing signedness breaking sign extension at the input of Instance? Plus cats propagating too much?
<Wanda[cis]>
hey, cats can propagate as much as they damn want
<_whitenotifier-7>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 94656c8 - Deploying to main from @ amaranth-lang/amaranth@6d65dc1366da2313e8e6a77d5093ddd6acdec8aa 🚀
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 8203827 - Deploying to main from @ amaranth-lang/amaranth@890e099ec3450306bc841311365d09e932d1b46f 🚀
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] d06063b - Deploying to main from @ amaranth-lang/amaranth@c40cfc9fb56e51d3b3fd96e3ccfb296d57e75456 🚀