<_whitenotifier-8>
[amaranth-lang/amaranth-lang.github.io] whitequark 6e3e264 - Deploying to main from @ amaranth-lang/amaranth@a70cfa05cd0052c70a1dc97c4659e006fc72d855 🚀
<whitequark>
there will be a meeting today (hooray!)
<whitequark>
this is the last RFC in the "aggregate data structure" RFC series and in my view it completes the whole arc
<whitequark>
the idea here is that instead of having data.View know about Signal in its constructor (whereas every other part of data.View only cares about wrapping a value), Signal would know to __call__ a shape-castable you give it
<whitequark>
please respond with your comments and proposed disposition: merge, close, or postpone
<Chips4MakersakaS>
merge
<jfng[m]>
the last drawback (Signal() not necessarily returning signals) stings a bit
<whitequark>
it does, yes
<jfng[m]>
otoh, most of the time i've had to call isinstance(x, Signal) was to deal with Records awkwardness
<jfng[m]>
so unsure if this is really going to be an issue going forward
<_whitenotifier-8>
[amaranth-lang/amaranth-lang.github.io] whitequark 0838816 - Deploying to main from @ amaranth-lang/amaranth@e9975587bf2fa6cfcc82db438ca2b938f07e5cb6 🚀
<whitequark>
in short, ValueCastable and ShapeCastable are protocol classes: they don't have any behavior in themselves but rather they mark another class as supporting a certain behavior
<whitequark>
it would be very nice if for every type we call "value-castable" in documentation, `isinstance(x, ValueCastable)` returned `True`
<whitequark>
fortunately Python already provides a facility to do this: ABCMeta
<whitequark>
the proposed semantics is: isinstance(x, Value/ShapeCastable) == True iff Value/Shape.cast(x) would succeed
<whitequark>
your comments?
<jfng[m]>
it's a clear improvement over the current "Value.cast() and find out" alternative
<jfng[m]>
merge
<Chips4MakersakaS>
merge from me too
Guest123 has quit [Quit: Client closed]
<whitequark>
adamgreig?
<adamgreig[m]>
yea, happy to merge
d7anna has quit [Read error: Connection reset by peer]
<whitequark>
okay, merge for issue #753
<adamgreig[m]>
I guess "merge" isn't quite right for an issue but merge the PR that it describes anyway
<whitequark>
the issue states it all so I won't copy the text to IRC
d7anna has joined #amaranth-lang
<adamgreig[m]>
for that, would you keep the hdl types like Elaboratable, Module, Signal etc accessible from top-level amaranth package and just deprecate using them from amaranth.hdl?
<jfng[m]>
will Value/ShapeCastable be included in the set of names ?
<whitequark>
it's slightly different
<adamgreig[m]>
could you just make the hdl module _hdl and not have to rename everything inside it?
<adamgreig[m]>
having modules start with _ is a bit unusual I guess
<whitequark>
you get the full set of programmer usable names exported from amaranth.hdl, a subset of it exported from amaranth (the prelude) and everything else is hidden
<adamgreig[m]>
ah, ok
<whitequark>
basically there's no reason for a programmer to care that "ValueCastable" lives in "ast" (it's not even really an AST)
<adamgreig[m]>
so you'd continue to support using the names that are meant to be public from amaranth.hdl, you'd keep the most used things in the prelude as now, but you just rename the non-public things?
<adamgreig[m]>
yea
<whitequark>
the prelude does not change
<whitequark>
a few more names get exported from amaranth.hdl (ValueCastable and ShapeCastable) for example
<whitequark>
I have one comment on the implementation, which is that everywhere else in core Amaranth we call the reset signal "rst"
<whitequark>
so for consistency this module should use "rst" too
<adamgreig[m]>
do I recuse myself for this one? anyway, no problem renaming the reset signal
<whitequark>
adamgreig: I voted against my own RFC once, so :)
<whitequark>
oh, no, I actually have another comment
<whitequark>
it would be nice to be able to use the same set of parameters for computing the CRC in Python
<whitequark>
this would be very usable in Glasgow for example
<adamgreig[m]>
along with the rest of the current std lib, I expect we might one day change the interface when all the new things land, but I expect that will be a while away
<adamgreig[m]>
hm
<adamgreig[m]>
so the _SoftwareCRC class that's in the PR does exactly that
<adamgreig[m]>
it's not the most efficient software implementation ever because it doesn't have an option to use lookup tables and it does some bit reversing by dumping words to strings and flipping them, which is fine for the one-off use in amaranth as part of generating the hardware
<adamgreig[m]>
and does work in software of course, it's just you could be quicker with a few fairly easy tweaks
<adamgreig[m]>
but I made it a private implementation detail of the module because it didn't seem obvious that people would want to use it or that we'd want to maintain this software crc generator
<adamgreig[m]>
it wasn't really a popular package when I wrote this at first, but since then it's got a new maintainer(?) and has a bunch of releases
<adamgreig[m]>
it could be used directly in amaranth to compute the equations the hardware needs, but it would add a dependency to amaranth
<whitequark>
I think we're fine directing people to this package in the documentation
<adamgreig[m]>
we could update the parameter names in amaranth to exactly match that library, I suppose?
<adamgreig[m]>
they do all have the same meaning
<adamgreig[m]>
ah, it's a slight subset - the crc library only supports 8/16/32 bit wide CRCs, and only supports feeding byte-sized words
<adamgreig[m]>
whereas the amaranth one supports any crc width and input word width
<whitequark>
renaming the parameter names sounds good to me, width / data_width sound fine
<whitequark>
oh, what's the reason for inheriting CRC from _SoftwareCRC? you're not overriding any of the implementation, right?
<adamgreig[m]>
correct, here it's just a convenient way to get those methods. It's historical from an earlier version that also had a ROM based hardware version that also inherited from the software one
<whitequark>
changing the parameters on the fly isn't supported anyway (for at least m, n the sizes of the inputs/outputs won't get changed)
<adamgreig[m]>
the methods could all just go into the main class
robtaylor has joined #amaranth-lang
<whitequark>
Rob Taylor: the meeting is just about to end
<adamgreig[m]>
thinking about it, I have a slight preference for the current "ref_in"/"ref_out" names vs `crc`'s "reverse_input"/"reverse_output" because I think reflection is a slightly better word for it, but it's not especially important and lining up with `crc` might be more useful
<whitequark>
hm
<robtaylor>
Catherine: just realised I'd somehow got disconnected from this room :(
<adamgreig[m]>
the IRC bridge will kick idle users from the matrix side
<adamgreig[m]>
as I discovered after getting back from holiday :P
d7anna has quit [Remote host closed the connection]
<adamgreig[m]>
in principle just reading the messages (with read receipts on) should be enough to indicate activity, not actually talking
d7anna has joined #amaranth-lang
<whitequark>
we've ran out of time; let's decide to rename the reset signal and leave the rest for the next meeting?
<whitequark>
I want a bit more time to think about method names
<whitequark>
err, parameter names
<adamgreig[m]>
sure, I'll update the PR for reset
<whitequark>
thank you!
<whitequark>
I might send a PR for the other changes even
<whitequark>
okay, this concludes the meeting. thanks everyone! we've progressed a lot in the last few months. with RFC 15 approved I would consider aggregate data structures complete; I'll put the finishing touches on the feature today and everyone is encouraged to use it
<whitequark>
anyone going to port minerva to it? :)
<jfng[m]>
when i get the time !
<jfng[m]>
though i'd rather wait for interfaces and maybe streams before doing a big refactor
<whitequark>
sounds reasonable
<whitequark>
there is value in starting to use data structures early to catch any issues a while before releas
<whitequark>
s/releas/release/
<jfng[m]>
also, is there a plan for exposing data structure fields to e.g. a .gtkw file ?
<robtaylor>
<adamgreig[m]> "the IRC bridge will kick idle..." <- doh!
<whitequark>
jfng: there isn't and I'm not sure it's feasible
<whitequark>
we would probably have to special-case Layout in all the backends or something
<jfng[m]>
because their absence would make debugging tedious (e.g. the pipeline control path may have dozens of fields in its layout)
<whitequark>
yeah
<whitequark>
we could definitely add a custom decoder to Layout
<whitequark>
and I guess define a decoder protocol, like say that if it returns a dict, it's splatted out into that many subsignals
<cr1901>
Ahhh shit, I overslept. Oh well, I'll catch up on the backlog, I'm sure it's fine.
<whitequark>
for CXXRTL this would have to use VCD post-processing
<jfng[m]>
what would be the keys of this dict ?
<whitequark>
names of subfields
<whitequark>
tbh, we probably need to deprecate .decoder and its call only protocol and add something like .formatter that uses the (in the process of being added) Print() statement format strings so that CXXRTL could format on its own
<_whitenotifier-8>
[amaranth-lang/amaranth-lang.github.io] whitequark 748400f - Deploying to main from @ amaranth-lang/rfcs@f747603cbf92ebdb628a62e603df47032728ee1c 🚀
<jfng[m]>
could this be done at the RTL level ? e.g. for each field, create a wire with a "keep" attribute
<jfng[m]>
just so it appears in a VCD
<whitequark>
in principle yes, but we need a way to communicate this from lib.data to back.verilog
<whitequark>
and that part is annoying
<jfng[m]>
yeah, using a amaranth-specific attributes
<jfng[m]>
.. or not ?
<whitequark>
I don't like the idea of using attributes specific to lib.data alone since then people will want to use them themselves and once again reach into the implementation details
<_whitenotifier-8>
[amaranth-lang/amaranth-lang.github.io] whitequark c158af8 - Deploying to main from @ amaranth-lang/amaranth@7d99981d571000fb427f051f1bbe9631c1a44145 🚀
<whitequark>
agg, you worked with fixed point, right?
<whitequark>
which API would you expect an Amaranth fixed point view to have?
<whitequark>
add, sub, neg, .int, .frac, what else?
<whitequark>
* __add__, __sub__, __pos__, __neg__, .int, .frac, what else?
<agg>
Something to get at the size of the int and frac bits and signedness, mult, bit shift left/right, probably some sort of truncate/resize operation, saturate, hmm
sugarbeet has quit [Ping timeout: 268 seconds]
<agg>
i think it's fairly common to have like Q3.4 and want to either truncate to like Q3.2 (loses information but can't overflow), or Q1.4 (with either saturating or wrapping, sometimes you know the integer part will be small enough so don't care and wrapping is ok, other times you want to saturate because you're not sure)
sugarbeet has joined #amaranth-lang
<agg>
probably something to enlarge as well
<whitequark>
Q3.4?
<whitequark>
oh, in bytes?
<agg>
ah sorry, just made up example sizes
<agg>
Q notation is "QN.M" where n is the number of integer bits and m is the number of fractional bits, plus one signed bit
<agg>
UQ for unsigned, and if N is 0 you usually just write QM
<agg>
so Q15 is a 16-bit word where 1 bit is sign, 0 integer bits, 15 fractional bits, -32768 represents -1.0 and 32767 represents 0.99996...
<agg>
Q5.10 is a 16 bit word too, but now instead of -1 to 1 it's -32 to 31.999
<whitequark>
thank you
<agg>
if you had Q3.4 that's an 8-bit word for -8 to 7.9375, but maybe you want to throw away two fractional bits to get Q3.2, or maybe you know you're actually in the range -2 to 1.9375 and you want to remove the top integer bits
<agg>
you commonly want to change sizes, or sizes are changed because you've done some operation
<whitequark>
right, yes
<whitequark>
the idea is that sizes would change automatically to avoid overflow/truncation with any operation
<agg>
yea
<agg>
which is good, but then you want some operations to subsequently truncate
<whitequark>
right
<agg>
mult is definitely a thing to have, and especially with small words it's not too awful in fpgas, but of course it does blow up the size of the output word
<agg>
and if you're doing like mult+accumulate you often want a quite wide accumulator
<whitequark>
right okay
<agg>
oh, you'd probably want some easy way to grab the whole word as a single Signal too, but maybe that's obvious
<agg>
like i'd love to have amaranth know that values are fixed point and track simple operations, but then i'm probably going to shove them into the hard dsp block and then tell amaranth the output has a particular format too
<agg>
getting the sign bit out separately would be nice for that too since the dsp blocks often have independent sign
<whitequark>
oh, like an accessor for the sign bit?