<ZhouZhang56>
Hello, thanks for producing such a wonderful tool. The version 0.3 is released almost 2 years ago. When would a new version (0.4) be released?
<whitequark>
as soon as feasible. I was recently hired by ChipFlow and am now working full-time to bring version 0.4 and other development forth
<whitequark>
in terms of features, you can look at the 0.4 milestone on the issue tracker; Interfaces and Streams are the key parts, and there are some auxiliary features
ZhouZhang56 has quit [Client Quit]
ZhouZhang92 has quit [Ping timeout: 245 seconds]
ZhouZhang35 has joined #amaranth-lang
<whitequark>
in terms of time, hopefully a month or so from now?
<ZhouZhang35>
Great! Thanks!
<whitequark>
no problem
<d1b2>
<Darius> congrats on the job
<whitequark>
thanks! it's been instrumental in me getting stable housing, away from geopolitical conflicts, and with healthcare
jjsuperpower has joined #amaranth-lang
jjsuperpower_ has joined #amaranth-lang
ZhouZhang35 has quit [Quit: Client closed]
jjsuperpower has quit [Ping timeout: 240 seconds]
jjsuperpower__ has joined #amaranth-lang
jjsuperpower_ has quit [Ping timeout: 268 seconds]
jjsuperpower__ has quit [Ping timeout: 240 seconds]
<cr1901>
amaranth doesn't have popcnt, correct?
<charlottia>
Not as far as I’ve seen!
<adamgreig[m]>
cr1901: iirc you can just do sum(signal)
<adamgreig[m]>
since signal iterates over its bits and sum reduces its argument with addition, you end up with an adder of each bit
<cr1901>
oh cool, good to know
Lord_Nightmare has quit [*.net *.split]
lethalbit has quit [*.net *.split]
alanvgreen has quit [*.net *.split]
yuriks has quit [*.net *.split]
alanvgreen has joined #amaranth-lang
yuriks has joined #amaranth-lang
lethalbit has joined #amaranth-lang
Lord_Nightmare has joined #amaranth-lang
indy has quit [Ping timeout: 240 seconds]
indy has joined #amaranth-lang
<d1b2>
<Olivier Galibert> Are there currently recommendations for unit testing of elaboratables? Let's say I implement a cpu, I want to unit-test it staying on the python side, and the test ends up being a bunch of programatically generated traces and things of the kind. Possibly using python's unittest, since it seems to be the standard?
<d1b2>
<bob_twinkles> pytest + amaranth's sim backend seem to work pretty good, though I haven't tried like mocking or anything
<adamgreig[m]>
Another vote for pytest, I find it much much nicer to write tests with than unittest. And then yes, pysim backend for simple tests, or I have pytest build a test runner with yosys cxxrtl and run that too for things that need to be faster
<whitequark>
yeah, right now it's basically that
<whitequark>
eventually I'll get around to writing a manual
FL4SHK has quit [Ping timeout: 256 seconds]
<Chips4MakersakaS>
I will not make it to the IRC meeting today if there will be one.
<whitequark>
there will be one
<whitequark>
should we reschedule?
<Chips4MakersakaS>
Not for me.
<whitequark>
sorry?
<Chips4MakersakaS>
You don't need to reschedule the meeting for me. It's just that it's holiday basically in whole Europe and America.
<whitequark>
oh, makes sense
<whitequark>
what I'm thinking of for a while is that we may need to reschedule it for the time to be more convenient to all participants
Sarayan has joined #amaranth-lang
FL4SHK has joined #amaranth-lang
toshywoshy has quit [Ping timeout: 265 seconds]
ktemkin has quit [Ping timeout: 265 seconds]
ktemkin has joined #amaranth-lang
toshywoshy has joined #amaranth-lang
<FL4SHK>
whitequark: do you have any suggestions for how to use cxxsim?
<FL4SHK>
I now have a module I'd like tot est
<FL4SHK>
actually, maybe I'd better hold off until my CPU is complete
cr1901 has quit [Remote host closed the connection]
cr1901 has joined #amaranth-lang
cr1901 has quit [Remote host closed the connection]
<adamgreig[m]>
I've implemented the changes we discussed last week
<adamgreig[m]>
though I didn't end up with a SoftwareProcessor class because it ended up being just one method that might as well live on Parameters
<whitequark>
I do like the new APIs. I have some comments on it, relatively minor ones
<whitequark>
the first one is that params1 = crc.Parameters(8, 8, 0x2f, 0xff, False, False, 0xff) is yearning for keywords, and Parameters should probably talk only keyword arguments
<whitequark>
(def __init__(self, *, x, y, ...))
<adamgreig[m]>
sure, sounds good
<whitequark>
the second one is that instances of Predefined are literally just closures, right?
<adamgreig[m]>
sort of
<adamgreig[m]>
the difference to Parameters is 1) they don't have data_width, which is where they could be like a partially applied Parameters initialiser
<adamgreig[m]>
but also 2) they contain the check and residue values from the catalogue, which we use just to compare to software-computed values in the unit tests
<adamgreig[m]>
those values could move to a table in the unit tests instead, of course, there's no practical reason for end users to need them
<whitequark>
ohh, they have the residue
<adamgreig[m]>
in that the Parameters class has a method to compute the check and residue values
<adamgreig[m]>
so end users can just call that method on any Parameters, which they can obtain from a Predefined readily enough
<adamgreig[m]>
but since I was converting the entire catalogue into python data structures it was easier to just keep all the data from the catalogue in one place
<d1b2>
<Olivier Galibert> sorry I'm late, hi
<whitequark>
hi Sarayan
<adamgreig[m]>
still, you could delete Predefined and instead have Catalog just be full of partially applied Parameters with data_width missing, and a separate table in the unit tests
<adamgreig[m]>
I don't see much use of partial application in python really though
<adamgreig[m]>
it's possible I've slightly misunderstood your original question about them being closures actually
<adamgreig[m]>
they are closures in the sense that they implement call and you call them to obtain the class
<adamgreig[m]>
s/call/`__call__`/
<jfng[m]>
could Predefined inherit from Parameters, but with data_width=None, and Parameters would have a data_width property that would raise an AttributeError if no data_width is given ?
<jfng[m]>
so all the methods that require one would fail
<adamgreig[m]>
what would be the advantage?
<jfng[m]>
i feel like predefined should be an instance of Parameters, after all, that what they are
<whitequark>
I'm not sure it's all that compelling one way or another
<adamgreig[m]>
I'd like if you could easily get a fully-defined parameters from them without having to like, copy something from the catalog and then tweak its data_width attribute
<whitequark>
yeah
<whitequark>
I think having Predefined live in its own corner as basically a table is fine
<whitequark>
it will be rarely useful to pass around on its own, but I can actually see that happening, and I can see interrogating its attributes being useful
<whitequark>
so making Predefined equal functools.partial, my original suggestion, is probably not that good of an idea
<whitequark>
well, consider a streams based CRC generator that is parameterized over (a) data width and (b) CRC algorithm
<whitequark>
I guess you could make it accept Parameters already, but what if it's a part of a bigger block that is still parameterized by the type of CRC?
<adamgreig[m]>
it could always just change the data_width in the Parameters
<whitequark>
something like a SERDES wrapper
<adamgreig[m]>
(though I'm not a huge fan of the suggested API being 'tweak attributes', I guess)
<adamgreig[m]>
but I don't have it enough to have made all those attributes read-only or anything
<adamgreig[m]>
s/have/hate/
<whitequark>
I'm arguing in favor of the API you have in the PR already
<whitequark>
i.e. separate Parameters and Predefined
<adamgreig[m]>
yea
<adamgreig[m]>
I think it's fine. we could probably also design it so that it's just Parameters but not sure it's compelling either way
<whitequark>
how about renaming Predefined to Algorithm?
<whitequark>
Algorithm is generic over data-width, Parameters (for a Processor) are not
<adamgreig[m]>
and then Parameters would have to take Algorithm + data_width?
<whitequark>
yes
<adamgreig[m]>
constructing a Parameters becomes pretty awkward at that point?
<adamgreig[m]>
if you're not using a predefined one, anyway
<adamgreig[m]>
I assume if you're using a predefined one you just call crc.Catalog.CRC8_CCITT(data_width=8)
<whitequark>
right
<adamgreig[m]>
p = crc.Parameters(crc.Algorithm(crc_width=8, ...), data_width=8), I guess it's not that much worse but it's a bit wordier
<adamgreig[m]>
mostly it seems a bit contrived, Parameters naturally includes all those other details
<adamgreig[m]>
still, it is annoying that data_width is the only one that can't be stored in the catalogue
<whitequark>
tbh I was thinking of just renaming it originally
<d1b2>
<Olivier Galibert> you can default it to the crc width
<whitequark>
sorry, let me try to address things one by one
<adamgreig[m]>
Olivier: I don't think that's usually right, it's very common yo have a CRC16 or CRC32 being computed over bytes for instance
<d1b2>
<Olivier Galibert> hence "default", not require
<whitequark>
so the first one is that I think there is genuine separation between whatever's called "Predefined" and whatever's called "Parameters". the former is a computation and a family of algorithms, the second is a concrete instantiation of an algorithm
<whitequark>
like between a class and an instance
<d1b2>
<Olivier Galibert> Could "crc.Catalog.CRC8_CCITT" be a factory that gives you a Parameters?
<whitequark>
it's already that
<d1b2>
<Olivier Galibert> (and by factory I mean raw function)
<whitequark>
adamgreig: consider that having a fixed list of "Predefined" where we have e.g. "CRC8_CCITT" and "CRC8_CCITT(data_width=8)" gives you an instance of something you call methods on
<whitequark>
I'm not suggesting using inheritance to define individual algorithms, but there is a parallel
<whitequark>
I'd also like to have Catalog not a class, since it makes no sense to have instances of it
<whitequark>
* Catalog not be a class,
<whitequark>
should be either a dict or a module, and a module is way more ergonomic
<whitequark>
you will actually be able to do from amaranth.lib.crc.catalog import CRC8_CCITT and then CRC8_CCITT(data_width=8) since you don't need any other objects if you're using predefined algorithms
<whitequark>
yep, I think it should definitely be a module and not a class
<whitequark>
any objections?
<adamgreig[m]>
yea, sounds good, I wasn't a huge fan of it being a class
<whitequark>
okay, so we're on the same page there
<whitequark>
about Predefined, there are two separate possible changes: one just to rename it to Algorithm (pure bikeshedding, feels a bit nicer to me)
<whitequark>
and the other is to pass instances of Algorithm around instead of copying all the details into Parameters
<cr1901>
No objections, but how is `CRC8_CCITT(data_width=8)` a dict or a module (it has the syntax of a function)?
<whitequark>
the latter lets us have type checking happen once in Algorithm
<adamgreig[m]>
cr1901: `catalog` is the module, CRC8_CCITT is an instance of a class that implements `__call__()`
<d1b2>
<Olivier Galibert> Why so complex?
<cr1901>
oooooh, alright
<d1b2>
<Olivier Galibert> what about:
<jfng[m]>
we aren't there yet, but i think the assert isinstance(parameters, Parameters) should be an if not isinstance(...): raise TypeError(...)
<whitequark>
yes
<jfng[m]>
in Processor.init
<whitequark>
we can hash that out when the RFC is merged
<adamgreig[m]>
Catherine: happy to rename to Algorithm, once you do that I can see why you'd have Parameters take it instead of the individual values, my only slight annoyance is that constructing a plain Parameters from raw values becomes a bit wordy but the most I think about it the less I care
<adamgreig[m]>
I'd still have Algorithm be callable with data_width to construct Parameters though, I suppose
<whitequark>
adamgreig: the main thing I can see being annoying about it is the additional level of indirection when using it, but... the crc module is so small, and will be touched so rarely
<adamgreig[m]>
well, it's that or a method that does the same thing but has a name
<whitequark>
I am 100% in favor of Algorithm being callable
<whitequark>
that works out well in the class/instance analogy!
<adamgreig[m]>
indeed
<adamgreig[m]>
jfng: yea, makes sense. shame that's not a more built-in python thing perhaps
<adamgreig[m]>
OK, so s/Predefined/Algorithm, move catalog to a module, have Parameters take an Algorithm on construction so all type checking happens in Algorithm, but keep Algorithm callable and keep the check/residue values in it?
<d1b2>
<Olivier Galibert> 100% customizability, trivial for IDEs to parse, trivial for normal python users to understand
<adamgreig[m]>
one possible downside to keeping the check and residue values in Algorithm is that you now need to know them to construct it
<whitequark>
Sarayan: then you can't do CRC8_BLUETOOTH.crc_width, meaning that you cannot pass bare CRC8_BLUETOOTH around
<d1b2>
<Olivier Galibert> You can pass references to functions around
<adamgreig[m]>
more importantly, you never never want to write CRC8_BLUETOOTH(polynomial=0x7a)
<whitequark>
also that, yes
<adamgreig[m]>
if anything it should be def CRC8_BLUETOOTH(data_width=8): return Parameters(8, data_width, 0xa7, ...)
<adamgreig[m]>
but yea, no need for it to be a function when it can be an instance that can have attributes
<whitequark>
what adamgreig just wrote was my first thought on this (see above in the log when I talked about `functools.partial`)
<d1b2>
<Olivier Galibert> You're 100% sure you'll never ever want to customize reflect_* or xor_output?
<adamgreig[m]>
if you do, it's no longer "CRC8_BLUETOOTH"
<d1b2>
<Olivier Galibert> so what?
<adamgreig[m]>
it's some other algorithm now, not compatible
<whitequark>
but this is strictly less useful because you can't interrogate a function for the values it passes to something else in code
<adamgreig[m]>
if you want some other algorithm, you can spell it out yourself, or even instantiate CRC8_BLUETOOTH and then change the relevant attributes
<d1b2>
<Olivier Galibert> Mhhh, I'm reaaly think you're on the wrong side of KISS there
<whitequark>
I don't follow KISS
<d1b2>
<Olivier Galibert> I consider it's a guideline, but a good one
<whitequark>
(nor do I follow "Zen of Python", or other similar principles)
<whitequark>
I've mentioned to you why I decided against keeping predefined algorithms functions
<d1b2>
<Olivier Galibert> You did? Sorry I must have forgotten
<whitequark>
twice, in fact
<d1b2>
<Olivier Galibert> Fuck, I need a new brain, the current one is broken then
<adamgreig[m]>
Catherine: what about keeping check/residue in Algorithm vs moving them to just live in the unit tests? I hadn't considered before when you'd never be creating your own Predefined, but if all custom crc goes through Algorithm it's probably a bit annoying to be required to specify them
<whitequark>
adamgreig: lgtm on moving them to tests, especially the check value which feels a bit weird to me calling it the de-facto standard (I've never heard of it before)
<adamgreig[m]>
my feeling is to move them to a table in the unit tests only, because that's all they're used for
<whitequark>
the residue is calculated from parameters so as long as the Parameters.residue function works and the parameters are correct, the user of the CRC library will need it, right?
<whitequark>
it's just something we use to verify our implementation
<adamgreig[m]>
that's right
<adamgreig[m]>
(the user of the crc library won't need it)
<whitequark>
yeah I think both of those are best moved to tests
<d1b2>
<Olivier Galibert> good thing with moving to tests, you can check some algorithms with multiple values
<d1b2>
<Olivier Galibert> in case there's a reason for that
<adamgreig[m]>
there shouldn't be; for this catalogue of algorithms the only test vector we have for each is that "123456789", any others I'd just be computing the result myself anyway
<adamgreig[m]>
but in any event it suffices to be sure all the parameters are correct
<adamgreig[m]>
* but in any event that check suffices to be sure all the parameters are correct
<whitequark>
the semantics of first seem OK
<adamgreig[m]>
I like the semantics, I'm not in love with the extra mux in the comb path
<whitequark>
yes, there is an added mux, but if first xor valid, then I think the synthesizer should be able to fold it into the reset?
<adamgreig[m]>
if it was last instead of first, or reset, you wouldn't need it
<adamgreig[m]>
I haven't tested, but yea, hopefully
<adamgreig[m]>
certainly no reason it shouldn't be able to see that
<whitequark>
if someone really wants to avoid the mux, they can ignore first and instantiate Processor with ResetInserter
<whitequark>
it's a straightforward local change that, if it really becomes an issue, we can even put into a note in docs
<whitequark>
I think we won't be committing to anything particularly bad by having this API
<whitequark>
last is just synchronous reset, right?
<whitequark>
wait, no
<adamgreig[m]>
last is a bit weird, because it implies you'd like a valid output on the next clock cycle but it's not clear when the reset then happens
<d1b2>
<Olivier Galibert> last would trigger hold time issues I suspect
<whitequark>
I feel like Sarayan is onto the right idea
<whitequark>
consider that for both first and last, the problematic case is having them asserted together with valid where valid is always on
<jfng[m]>
should first really be allowed to have side-effects when valid isn't asserted ? wrt. stream semantics
<whitequark>
jfng: Processor isn't exporting an instance of stream right now
<whitequark>
so it's just its own semantics
<d1b2>
<Olivier Galibert> "stream" isn't implemented at this point anyway, right?
<whitequark>
when converting it to streams, we can always add a gasket, regardless of the choices we make now
<jfng[m]>
ok
<whitequark>
but otherwise it is a good point
<whitequark>
we could replace all uses of `first` with `first & valid~
<whitequark>
s/`/\`/, s/~/`/
<adamgreig[m]>
you could call it 'start' instead of 'first'?
<adamgreig[m]>
but I'm sure I've often wanted to restart the crc at a different time to the first valid data
<whitequark>
I don't have an incredibly strong opinion on first so I'd like to hear from others
<whitequark>
cr1901: something that softens the point of contact between two parts; APIs here
<whitequark>
it's just a word I've used here metaphorically
<cr1901>
Ahhh
<cr1901>
Well first isn't in AXI Stream spec, I will probably be targeting AXI Stream, so I wouldn't be using first anyway
<d1b2>
<Olivier Galibert> Ok, one important point I suspect, while valid is eventually equivalent to EnableInserter, start/first and sync-ResetInserter are not
<whitequark>
that's another thing a gasket would be doing
<whitequark>
between AXI Stream and Amaranth streams, provided Amaranth streams have both first and last (which isn't clear yet)
<cr1901>
First can be inferred in AXI Stream; only (t)last is provided
<cr1901>
But ack
<adamgreig[m]>
if you have last you can just wire that to first through a flop and everything should work out
<jfng[m]>
i (vaguely) prefer `first` over `last`, as the latter makes it the responsibility of the previous user to properly clear it
<jfng[m]>
whereas by asserting `first`, you don't have to think about what happened before
<cr1901>
Amaranth will need some way to make sure "if last was asserted on the last xaction, first must be asserted on this one"
<whitequark>
just to make it clear, we're not discussing streams yet, stay on topic please
<adamgreig[m]>
(I mean: if the CRC unit has first, and your stream only has last, you can m.d.sync += crc.first.eq(stream.last))
<cr1901>
Ack
<adamgreig[m]>
OK, well if we keep first as-is with same name and semantics then I've not got any other comments
<jfng[m]>
(i'm not talking of streams here)
<whitequark>
got it
<adamgreig[m]>
unless anyone can think of a clever way to tweak the logic to avoid the mux entirely :P
<whitequark>
in that case, I think I agree with JF
<whitequark>
I don't think the mux is avoidable
<whitequark>
I think having first do things when valid is not set isn't inherently violating anything stream-related?
<whitequark>
the CRC Processor would have an input stream and an output stream
<d1b2>
<Olivier Galibert> does the module work correctly with a ResetInserter?
<cr1901>
I'd rather have last, but we can discuss a gasket during the streams discussion. So don't block the decision on me.
<whitequark>
so one valid word on the input one would provide one valid word on the output one
<adamgreig[m]>
presumably the stream can't assert first if not valid anyway, so it shouldn't matter?
<d1b2>
<Olivier Galibert> cr1901: last means you don't know when the data output is going away
<whitequark>
yeah
<d1b2>
<zyp> it's easier to infer first from last than the opposite
<d1b2>
<Olivier Galibert> and if you maximize hold time... you end up inferring first from last in the crc module
<adamgreig[m]>
yea, last is quite awkward for the crc
<whitequark>
I think keeping first with the existing name and semantics is okay; any objections?
<jfng[m]>
hmm, maybe rename it to start then ? to put an emphasis on the fact that it may be used independently of valid ?
<cr1901>
No objections (As in: not my favorite, but I'll get over it).
<d1b2>
<Olivier Galibert> you mean first acts at the next enable and not the next valid?
<whitequark>
it sounds like both jfng and cr1901 have objections
<jfng[m]>
first implies "this is the first word of data" imo
<adamgreig[m]>
I wouldn't mind start since it will be clear you can wire it directly to first if you have first, but you can also wire it one-after-last if that's what you've got, or just use it independently to restart the crc
<jfng[m]>
agreed
<whitequark>
start sounds good to me
<adamgreig[m]>
it seems like a slightly better fit for the semantics
<whitequark>
so, disposition: merge, with changes discussed?
<cr1901>
whitequark: My objection ("inferring first from last" semantics is what the big streaming interface spec does) can be solved w/ your aforementioned gasket
<cr1901>
renaming to start makes me feel better tbh
<whitequark>
ah
<cr1901>
So merge w/ changes discussed, wait for the streaming discussion to discuss first vs last
<cr1901>
(for me anyway)
<whitequark>
Sarayan: yes, the module works fine with ResetInserter
<jfng[m]>
merge
<d1b2>
<Olivier Galibert> ok, start/first not using valid feels a little weird, but heh
<whitequark>
adamgreig: oh, should we keep `Parameters.check` if we're getting rid of `Algorithm.check`?
<d1b2>
<Olivier Galibert> (we need a RFC/definition of resets at some point)
<adamgreig[m]>
I don't think there's much poitn
<adamgreig[m]>
the unit tests can just call compute(b"123456789")
<adamgreig[m]>
as can anyone else interested in verifying it
<whitequark>
Sarayan: first not using valid would be weird, start seems alright, I think, and agree re: resets
<adamgreig[m]>
it is a commonly used test vector, and I guess it doesn't hurt to have an extra method
<adamgreig[m]>
but anyone interested in it probably can call compute themselves
<adamgreig[m]>
having some concept of actual data flow will be really nice. maybe we can have an amaranth skid buffer and fifo stream adapter and such too
<jfng[m]>
adamgreig[m]: soon !
<adamgreig[m]>
i have like a kloc of various weird fifo adapters, heh
<whitequark>
I guess we could have Shape.of and some sort of __Shape_of attribute that's being returned by that method
<d1b2>
<Olivier Galibert> and yeah, that issue felt weird. Is Signal holding the Layout or is it holding the result of casting to Shape?
<whitequark>
with the invariant that Shape.cast(Shape.of(X(...))) == Shape.cast(X) or something like that
<whitequark>
the Signal is holding neither
<whitequark>
the Signal is just the Signal. but when you call Signal(layout), it returns layout(Signal(layout.shape()))
<d1b2>
<Olivier Galibert> What is Signal(...).shape then?
<whitequark>
a Shape instance
<whitequark>
ah, here we go
<d1b2>
<Olivier Galibert> Yeah, I think that's the root of the issue
<d1b2>
<Olivier Galibert> ideally, it should be the original layout, to make things really transparent
<whitequark>
actually, the View instances do not define .shape() at all
<d1b2>
<Olivier Galibert> (IMHO, I think, etc)
<whitequark>
right now they define no methods starting with a-z (and therefore let you create any field starting with a-z)
<whitequark>
this is a part of our ValueCastable contract: literally anything goes
<whitequark>
however, ValueCastable was made before ShapeCastable, extensibility, and so on
<d1b2>
<Olivier Galibert> Specically, I would find it nice and orthogonal to have Signal(something-shape-castable).shape return the something
<whitequark>
and before anyone really conceptualized that ShapeCastable : ValueCastable :: extensible types : extensible values
<whitequark>
yes, I think that makes sense
<whitequark>
I'll look into turning that into an RFC
<d1b2>
<Olivier Galibert> cool
<whitequark>
thanks Sarayan! that was useful
<d1b2>
<Olivier Galibert> 🙂
<d1b2>
<Olivier Galibert> you'd end up with an equivalence between Signal.like(x) and Signal(x.shape), I think
<whitequark>
yes
<d1b2>
<Olivier Galibert> which feels natural to me
<whitequark>
that's actually almost what it does right now
<whitequark>
Value.cast(other) will erase the ValueCastable nature, but if ValueCastable implements .shape(), then that's fine
<d1b2>
<Olivier Galibert> should it be .shape() or .shape?
<whitequark>
.shape() is currently a method on the entire built-in hierarchy
<d1b2>
<Olivier Galibert> I don't remember whether signals have shape or shape()
<whitequark>
mainly because if it is a deeply nested expression, it can result in a computation proportional to the expression size
<whitequark>
this was probably a bad reason, but it's what we have now
<jfng[m]>
btw, if i have a struct with a field called "eq", it would be inaccessible right ?
<whitequark>
yes, right now eq is the one exception
<whitequark>
it will be eq and shape after
<whitequark>
oh and as_value
<cr1901>
mainly because if it1 is a deeply nested expression, it2 can result in a computation proportional to the expression size
<cr1901>
what is it1 and it2 here?
<jfng[m]>
it would be nice if we could detect and warn those, if that's not too hard
<d1b2>
<Olivier Galibert> yeah, but as_value is a "internal" detail of ValueCastable implementation, the feeling I have is that Value and ValueCastable should be as identical as possible from the POV of a user of the thing
<whitequark>
cr1901: that doesn't display like it1 and it2 for me or in the log
<whitequark>
no idea why your client does that
<whitequark>
jfng: yes, can you open an issue please? or even a PR right away
<whitequark>
so with this change, we would have ValueCastable -> .as_value() -> Value, ShapeCastable -> .as_shape() -> Shape, ValueCastable -> .shape() -> ShapeCastable, Value -> .shape() -> Shape
<WilliamDJones[m]>
> mainly because if `it1` is a deeply nested expression, `it2` can result in a computation proportional to the expression size
<WilliamDJones[m]>
(code italics mine)
<whitequark>
it's actually a little more complex than that because .as_value() can return another ValueCastable but this is the idea
<WilliamDJones[m]>
Catherine: What is `it1` and `it2` in that sentence you said previously?
<d1b2>
<Olivier Galibert> oh, something I wanted to check, is m.d.whatever += [[x.eq(y)]] correct?
<whitequark>
William D. Jones: again, no idea, these do not have 1 or 2 in my client or in IRC. I think it's the bridge adding random characters
<WilliamDJones[m]>
Oh, they don't have 1 or 2. I added those
<whitequark>
oh.
<d1b2>
<Olivier Galibert> wq: He means "what is this it you're talking about exactly?"
<WilliamDJones[m]>
Basically, Idk what "it" is referring to in your sentence, so I gave them names to disambiguate
<whitequark>
.shape() taking O(n) time where n = subexpression size
<d1b2>
<Olivier Galibert> hmmm, there's a lot of weirdness in that implementation you inherited
<WilliamDJones[m]>
How does ".shape() is currently a method on the entire built-in hierarchy" fix this?
<WilliamDJones[m]>
(Actually, I'm not sure I understand what is meant by ".shape() is currently a method on the entire built-in hierarchy", but I'm only passively following the convo rn)
<cr1901>
back to IRC
<whitequark>
well it's implemented on every Value descendant
<whitequark>
and Operator.shape() computes operands[0].shape(), operands[1].shape(), every time you call it
<whitequark>
especially if you're constructing huge expression trees, calling .shape() on them can take nontrivial time
<d1b2>
<Olivier Galibert> it's amusing how much I would implement all that stuff differently if I were starting from scratch
<cr1901>
ooooh okay, so when you say "this was probably a bad reason", you mean "this was probably a bad reason [to implement shape like this]"
<d1b2>
<Olivier Galibert> probably same for you, maybe even worse
* cr1901
's English parser is broken today
<whitequark>
no, it's probably fine to have it as a property
<d1b2>
<Olivier Galibert> shape is the type of a value, of course it's a property 🙂
<whitequark>
Sarayan: no, I'm actually pretty happy with how it looks
<whitequark>
most of the changes I want to make are minor
<whitequark>
(and I'll make them with RFCs)
<whitequark>
well, in Rust you have type(x) which is a function
<d1b2>
<Olivier Galibert> good for you 🙂 I would have had the shape being a stored property of the Value, computed on construction
<whitequark>
errrr in Python, not Rust
<d1b2>
<Olivier Galibert> (and the expressions all unified in DAGs)
<whitequark>
the expressions are disunified because every one carries its exact source location
<whitequark>
which is a key part of Amaranth and a huge point of positive difference with Migen, which didn't bother doing anything for debuggability
<d1b2>
<Olivier Galibert> aliases?
<d1b2>
<Olivier Galibert> (yes, debuggability is important, agreed 100%)
<whitequark>
you could have a complicated two-level system of storing operands and such
<whitequark>
but it's probably not worth it because indirection also has a perf cost
<whitequark>
I don't really care that much about the implementation anyway; I'm just providing context for why originally .shape() was made a method
<d1b2>
<Olivier Galibert> not sure what you mean there, but that's ok 🙂
<cr1901>
Yea, I really don't understand where you're going w/ this, but I feel like it's not worth making you upset trying to explain
<whitequark>
well you could store the operands and operator in an inner struct, and have a reference to the inner struct and a source location captured in an outer struct
<whitequark>
but now every time you access operands from code, you need to unwrap the outer struct
<whitequark>
it has a smaller memory footprint due to deduplication, but a larger performance footprint due to added indirection
<d1b2>
<Olivier Galibert> oh, I would just have had a list of locations in the Value node
<d1b2>
<Olivier Galibert> instead of a single location
<whitequark>
I consider anything that makes debug information worse unacceptable
<d1b2>
<Olivier Galibert> agreed, I'm just not imaginative enough to see where it makes it worse
<whitequark>
ideally, the output Verilog has a 1:1 correspondence to the Python source code, except where Arrays are used (because that results in legalization)
<d1b2>
<Olivier Galibert> ahhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh I see
<d1b2>
<Olivier Galibert> you want the verilog to be usable
<whitequark>
the most common complaint about languages that compile to Verilog is that the Verilog is too hard to debug
<whitequark>
"your [expletive] code generator breaks, and I have to [expletive] debug it all day"
<whitequark>
which is a fair complaint
<whitequark>
this just means that the code generator must be able to explain itself at all times
<whitequark>
we're not quite there, actually, but we're a good part of the way there
<whitequark>
it's sure better than Migen.
<d1b2>
<Olivier Galibert> yeah, we have a philosophical difference there, for me it compiles to rtlil and verilog is just there when you don't have a full yosys chain afterwards
<d1b2>
<Olivier Galibert> I get where you're coming from mind you 🙂
<whitequark>
well yes, you want to cover your use case, I want to cover everyone's use case
<whitequark>
this, in my opinion, is the difference between "someone who designs a language" and "a language designer"
<whitequark>
and if you're primarily interested in Xilinx devices you're sure not using Yosys for it
<d1b2>
<Olivier Galibert> I need to buy a xilinx fpga and fix that 🙂
<d1b2>
<Olivier Galibert> (ok, Lofty is going to say that mistral is not there yet either)
<d1b2>
<Olivier Galibert> (they'd be right too)
<whitequark>
speaking with my FPGA user hat on, what's the competitive advantage of Intel devices? if I want something larger than ECP5 I'm probably getting a Xilinx device
<d1b2>
<Olivier Galibert> "mister" is the advantage
<whitequark>
that's... it?
<d1b2>
<Olivier Galibert> for a software guy that can't hack hardware at that level, it's a huge one
<d1b2>
<Olivier Galibert> the de10-nano is rather powerful for its price, and all the mister extension boards are nice in general
<cr1901>
whitequark: The retro HW ppl have learned _hard_ into Altera/Intel
<cr1901>
I'm not a fan of Mister being the "one true dev board"
<d1b2>
<Olivier Galibert> Mister is the only affordable one though afaict
<cr1901>
I wonder if it's because "kevtris uses Altera for his cores, so everyone in his periphery followed suit"
<d1b2>
<Olivier Galibert> I don't think you can have an equivalent to the de10-nano in the Xilinx world
<d1b2>
<Olivier Galibert> wq: yes, but iirc they're a lot more expensive for a de10-nano equivalent
<whitequark>
$149!
<cr1901>
(Why does a retro system need 110k LEs?)
<d1b2>
<Olivier Galibert> no hdmi
<cr1901>
and you still have to buy a DRAM board for many systems
<cr1901>
to emulate many systems*
<whitequark>
you have an FPGA right there
<whitequark>
surely you can drive an HDMI output
<d1b2>
<Olivier Galibert> then the cost goes up
<whitequark>
don't you have to upgrade DE10-Nano anyway?
<d1b2>
<Olivier Galibert> not necessarily
<d1b2>
<Olivier Galibert> there is a small number of cores that work on the bare nano
<whitequark>
> But if you want to go the DIY route, first you’ll need a DE10-Nano, which you can get for around $170. You can get one from one of the above shops, or direct from places like Digikey and Mouser. While much of the addons are optional, to get the broadest compatibility (read: Neo Geo, for example) you’ll want to start with the 128MB SDRAM, which you can get for around $60.
<whitequark>
okay, that's $230 already, I feel like an HDMI PMOD is going to cost you less than $81
<whitequark>
I always thought Terasic boards were unnecessarily overprived
<whitequark>
s/overprived/overpriced/
<d1b2>
<Olivier Galibert> (note that the design of the sdram board is catastrophic, signal-integrity wise)
<d1b2>
<Olivier Galibert> you'd need to add the sdram or equivalent too
<whitequark>
that Zynq board has 512MB of DDR3
<whitequark>
why do you need more?!
<tpw_rules>
the sdram is wired to the fpga and core instead of going through axi
<d1b2>
<Olivier Galibert> the nano has 1G of lpddr3. The latency sucks
<cr1901>
You might need true random access at the byte level for these cores
<d1b2>
<Olivier Galibert> there is no might there
<whitequark>
hm
<cr1901>
I don't see why an lpddr3 core can't do that
<d1b2>
<Olivier Galibert> especially for arcade, there's a fucklot of distributed roms and rams
<whitequark>
what's the DDR3 access latency on the Xilinx SoC boards?
<whitequark>
I've heard it's not particularly good
<tpw_rules>
it's also about fighting the arm core
<cr1901>
number of clock cycles has gone up, but the actual wall clock time of latency for each ram gen has gone down
<whitequark>
yes, but if you meet worst case latency, does it matter?
<d1b2>
<Olivier Galibert> cr1901: refresh cycle kills you
<whitequark>
the retro cores aren't particularly fast
<cr1901>
Split into two banks that keep copies of the ROM, or use the system's refresh signal
<cr1901>
(SNES exposes refresh)
<whitequark>
cr1901: no, it's more than that
<whitequark>
the DDR isn't wired directly to the FPGA
<whitequark>
it's wired to an AXI crossbar that the CPU is also connected to
<cr1901>
ahhh, wheee
<d1b2>
<Olivier Galibert> on the nano it's avalon and not axi iirc, but same
<whitequark>
it's the Processing System DDR
<cr1901>
Well, seems like "when cribbing from the original MiST board, the Mister devs decided to target de0 nano"
<cr1901>
and it's too much work to port to more boards
<cr1901>
(Yea, fun fact, there was a MiST board too. The dev stopped maintaining it after Mister and clones took off)
<d1b2>
<Olivier Galibert> yeah, because thanks to the university stuff by intel, it was significantly less expensive than equivalents
<whitequark>
mhm
<d1b2>
<Olivier Galibert> also, a new board would need to be significantly better to make it interesting, otherwise what's the point to redesign all the hardware
<whitequark>
I still see Terasic boards way too expensive for what they provide
<cr1901>
And I still think "One True Board" is lousy
<d1b2>
<Olivier Galibert> well, you can buy an analogue pocket instead
<cr1901>
No.
<d1b2>
<Olivier Galibert> less powerdul, but a lot more ram bandwidth with better latency
<d1b2>
<Olivier Galibert> it has something like five rams you can access in parallel. Kevtris does know where the problems are
<whitequark>
did kevtris made it?
<d1b2>
<Olivier Galibert> not by himself this time, but yes
<cr1901>
Is the main bitstream encrypted?
<cr1901>
Like it is on Analogue NT/Super NT
<tpw_rules>
just barely. they've opened it up now though
<d1b2>
<Olivier Galibert> don't think so
<tpw_rules>
(the bytes are bitswapped)
<d1b2>
<Olivier Galibert> oh fun
<cr1901>
hah... well, I guess maybe Analogue Pocket is a reasonable alternative then, and I retract my "No."
<cr1901>
That would also be a second "killer app" for Mistral
<tpw_rules>
yeah a bunch of people have ported mister cores to it. i got linux booted with litex but it doesn't really do anything
<tpw_rules>
(though the framebuffer does work)
<cr1901>
tpw_rules: But yea, my main problem w/ Pocket was "it's locked down except for a carve out for users to load their own cores into a locked down system". I'll take your word for it that it's not that bad
<d1b2>
<Olivier Galibert> what would a good Xilinx board with hdmi and enough ram I could buy for my personal fun?
<cr1901>
(compared to Analogue NT/Super NT)
<cr1901>
Arty S7-50?
<cr1901>
wait, no HDMI
<d1b2>
<Olivier Galibert> the "carve out" is the friggin' main fpga though
<tpw_rules>
the ui and stuff is locked down but the core fpga is a separate chip you can do whatever with
<d1b2>
<Olivier Galibert> so it's more than a carve out, honestly
<tpw_rules>
yeah, they even expose the jtag to it
<cr1901>
okay, but is your user core loaded using partial reconf or... oh
<cr1901>
nevermind, if JTAG is exposed then it doesn't matter
<tpw_rules>
yeah you get complete control over the main fpga. there's a bus in and out to another fpga for video and controller/rom data
<cr1901>
whitequark: Will changing shape to a property instead of method have any speedups when computing trees? (you said "no, it's probably fine to have [shape] as a property")
<d1b2>
<Olivier Galibert> cr1901: afaik they can't do partial reconfig, they don't have the connections for that
<d1b2>
<Olivier Galibert> jfng: $575, ouch
<d1b2>
<dusk> pity the ebaz4205 doesn't have HDMI (or other high-speed IOs) exposed
<jfng[m]>
its release price (just the board) was $200 in 2018
<adamgreig[m]>
urgh, moving the predefined items to crc.catalog but they are instances of crc.Algorithm means as-is you can't from amaranth.lib import crc; crc.catalog.CRC8_CCITT() because crc.catalog isn't imported in crc