nates93[m] has quit [Quit: Idle timeout reached: 172800s]
<whitequark[cis]>
chat, can you help me name a thing?
<galibert[m]>
Maybe?
<whitequark[cis]>
I'm designing an (upstream) record/replay system for CXXRTL. it consists of three components: cxxrtl::recorder (which streams state changes from a running design), cxxrtl::player (which reads state changes and applies them to a design), and cxxrtl::cassette (which spools the changes)
<whitequark[cis]>
how should I call the header file? `cxxrtl_<something>.h`
<zyp[m]>
so going with the theme, you've got a tape deck
<whitequark[cis]>
`<something>` should be reasonably self-explanatory and not be solely understandable within the wordplay
<galibert[m]>
cxxrtl_tracing?
<whitequark[cis]>
hm!
<whitequark[cis]>
that's not too bad! thanks galibert
<whitequark[cis]>
any other takers?
<galibert[m]>
note that _replay could be even better
<whitequark[cis]>
oh! good idea!
<zyp[m]>
I like replay
<whitequark[cis]>
thanks chat ^^
<galibert[m]>
Meow :-)
<galibert[m]>
(chat in french is cat in english)
<whitequark[cis]>
hah
nelgau has joined #amaranth-lang
_whitelogger has joined #amaranth-lang
_whitelogger has joined #amaranth-lang
ravenslofty[m] has quit [Quit: Idle timeout reached: 172800s]
nelgau has quit [Read error: Connection reset by peer]
<tpw_rules>
if you don't want to discuss without me that's cool, just wanted to fulfill my end of the bargain. documentation is of course lacking. but i'm hoping to at least apply cunningham's law a little bit
<jfng[m]>
hi! it is time for the weekly soc meeting
<jfng[m]>
there is now a PR implementing RFC 30, if you want to try out component metadata
<jfng[m]>
no SoC example yet; but i'm working on it next
<jfng[m]>
@libera_cr1901:catircservices.org iirc, you wished that we look at amaranth-soc code in your CPU design in this meeting ?
<jfng[m]>
otherwise, i guess it's Q&A again
<cr1901>
jfng[m]: Shit! I'm here I'm here... I forgot about "fall back" of standard time
<jfng[m]>
np
<cr1901>
jfng[m]: So yes, I made a SoC example for Sentinel. It sometimes fits into my desired target (icestick), sometimes doesn't. I suspect one area where resource usage can be improved is decoding.
<jfng[m]>
i think that you could save some logic by using a narrow CSR bus for your peripherals, instead of wishbone everywhere
<cr1901>
Oh... yea. That's a great idea
<jfng[m]>
e.g. your timer and uart probably don't need a 32-bit data width, and the CSR bus guarantees atomic accesses, so you could still access say, 32-bit registers over an 8-bit bus
<cr1901>
No they don't need 32-bit data width. Right now, I'm doing horrible things like "32-bit access, each byte controls a different register"
<cr1901>
Except for led_bus, I couldn't figure out how to use the sparse option to decoder.add(), so I got kinda desperate and did whatever until something stuck
<cr1901>
Peripherals aren't usually initiators (they can be), so my discussion a few weeks ago is "there should be an easy way to get a wishbone bus for responders without having to pepper flip/flipped all over the place". Reasonable ppl can disagree
<jfng[m]>
the convention for bus signatures is to orient them from the point of view of an initiator by default
<whitequark[cis]>
it's completely arbitrary. if you do the opposite you will end up peppering flip/flipped when working on initiators
<whitequark[cis]>
there is basically no choice that leaves everyone happy, so I think the core issue is that for some reason you end up needing to manipulate the bus direction that often
<whitequark[cis]>
I'm not sure why is that, so that's something that needs looking into
<cr1901>
At least while building up -soc peripherals, I think responders are gonna be more common. E.g. in this toy SoC, there are 4 responders vs 1 initiator
<whitequark[cis]>
RFC 1 isn't just for amaranth-soc
<whitequark[cis]>
it's something that defines the language itself
<cr1901>
(RFC 2?)
<whitequark[cis]>
er, yes, sorry, RFC 2
<cr1901>
Additionally, like you say "isn't just for -soc", I can't say whether initiators/responders will be more common in general.
<jfng[m]>
in a cpu core, initiators would be more common, for example
<cr1901>
I think what happened is basically "ugh, I want wb buses from responder POV, using flipped screws w/ my mental model, I know, I will go through a bunch of effort to avoid flipped" 1/2
<cr1901>
"using flipped screws w/ my mental model" <-- I need some time to think about why this is. I can't promise I'll have an answer immediately
<whitequark[cis]>
it doesn't really matter whether initiators or responders will be more common in general, we just need to pick a direction and stick to it
<whitequark[cis]>
any choice will make someone unhappy. you just deal with the fact that sometimes it's you.
* cr1901
nods
* cr1901
tries to draw a picture... gimme a min, please
widlarizer[m] has quit [Quit: Idle timeout reached: 172800s]
<tpw_rules>
jfng[m]: hi, did you see my comments earlier?
<jfng[m]>
yes, i gave a quick look to your usage of the register API, and it looks fine
<tpw_rules>
okay, thank you
<tpw_rules>
my two concerns are 1. how to pass the memory map to the signature annotation and 2. the fact that csr.Register is ultimately an elaboratable means you can never construct more than one of the component that holds it, right?
<tpw_rules>
sorry, csr.Field (the superclass of e.g. csr.field.RW)
<tpw_rules>
there are also those comments i made on the implementation PR
<whitequark[cis]>
> 1. how to pass the memory map to the signature annotation
<whitequark[cis]>
this should happen automatically by wishbone.Interface, without any action from your side
<whitequark[cis]>
> 2. the fact that csr.Register is ultimately an elaboratable means you can never construct more than one of the component that holds it, right?
<whitequark[cis]>
this is intentional, because the register may want to include some logic to handle transactions
<tpw_rules>
re 1. i'm using csr.Interface/csr.Signature, not wishbone anywhere. maybe i'm mixing it with wiring.Component incorrectly?
<tpw_rules>
jfng[m]: i guess i disagree that the csr_bus is inherently a complex signature. it is in implementation because of the memory map but for example the other busses there don't need that and work nicely with wiring.Component
<jfng[m]>
i meant that `SampleStream` is a component with a complex signature, that would benefit from having its own `Signature` subclass
<tpw_rules>
SampleStream is already a signature?
<jfng[m]>
doing so would give you a natural place to create the csr bus signature, assign it the memory map, then add it as a member to the component signature
<jfng[m]>
`SampleWriter`, my bad
<galibert[m]>
If that is a complex signature, I don't want to see what it will look like on actual complex stuff
<galibert[m]>
There is a grand total of five entries
<tpw_rules>
again, i don't get why the csr bus signature has the memory map in the first place. in my mind it would only have the bus data signals
<jfng[m]>
galibert[m]: where one member is a bus signature, which as a memory map that is bound at a later point
<jfng[m]>
has*
<tpw_rules>
like is that really part of a signature? connect doesn't do anything with it
<tpw_rules>
well maybe i'm wrong, would `connect` stop you from connecting things with different memory maps?
<jfng[m]>
tpw_rules: no, it wouldn't
<jfng[m]>
...i think
<galibert[m]>
tpw_rules: because they want the annotations in the signature (one of the numerous problems I have with the annotations) which means they can't commit it before elaboratable but that's too late (signature is already frozen at that point), so they need to have a dynamic annotation generator that is pretend-fixed but actually changes all the way until elaborate (the MemoryMap)
<jfng[m]>
galibert[m]: we are not talking about component metadata
<tpw_rules>
what do you mean by annotations here?
<jfng[m]>
we are talking about python variable annotations
<tpw_rules>
then i'm not sure that keeping the memory map in the signature is the right thing
<jfng[m]>
where do you think it should be ?
<tpw_rules>
at least from the perspective of something the user has to care about
<tpw_rules>
i don't know, it seems like an implementation detail to me
<galibert[m]>
Then I'm even more with tpw_rules , why is the MemoryMap in there, it has nothing to do with the upsteam connection and that's what supposed to be in the signature
<galibert[m]>
I'm missing something in the logic
<galibert[m]>
OTOH I am quite tired, that could explain
<galibert[m]>
Sorry if I'm too harsh, I don't want to be
<whitequark[cis]>
<tpw_rules> "again, i don't get why the csr..." <- the signature of the bus interface is its "type", which tells you how to interact with it. this involves how to physically interact with it (what is its structure) as well as how to logically interact with it (which waveforms to use)
<whitequark[cis]>
is that clearer?
<galibert[m]>
Catherine: Is the type of a decoder depending on what is connected downstream of it?
<galibert[m]>
Or is the type only the upstream connector part?
<tpw_rules>
i guess then i didn't expect combining the physical interaction and logical interaction and i'm not sure i agree with it right now
<tpw_rules>
the interface rfc does to some degree address this in that a signature describes "requirements" and the interface also describes "invariants"
<tpw_rules>
it seems reasonable for an Interface to have a memory map, but i'm not sure about Signature
<tpw_rules>
well, more reasonable. i'm not sure the split between Interface and Signature in my head is right
<galibert[m]>
well, at least currently it doesn't match my intuitive vision of a type vs. an instance of a type
<jfng[m]>
<galibert[m]> "Then I'm even more with tpw_rule..." <- Memory maps are attached to bus interfaces, to describe the resources that are accessible through them.
<jfng[m]>
Bus interfaces are created by `Signature.create()`; we attach the memory map to the signature, because we don't create interfaces explicitly; `csr.Signature.create()` will then pass the memory map to the created interface
<whitequark[cis]>
galibert[m]: it depends on what is connected downstream of it
<whitequark[cis]>
well, to be more precise, there are different options you can take
<whitequark[cis]>
you can not have any MemoryMap annotation at all, say because you don't believe in them. then it doesn't.
<tpw_rules>
jfng[m]: i get this, but again that seems like an implementation detail
<jfng[m]>
no, it's a fundamental mechanism of amaranth-soc
<galibert[m]>
I believe in memory maps, I think that's useful, but for me they're part of the instance, not of the type
<tpw_rules>
(or at least something between them)
<whitequark[cis]>
if they're a part of the instance, then you cannot slice your SoC at interface boundaries and be able to replace peripherals without rebuilding the entire thing from source
<whitequark[cis]>
if they're a part of the type, you can
<tpw_rules>
to me also Signature is a type and Interface is an instance
<galibert[m]>
In other words, in my view of the world, on the same type of bus all decoder.signature would be equivalent while each decoer would have its own decoder.annotation (or whatever)
<galibert[m]>
s/decoer/decoder/
<whitequark[cis]>
yes, I realize this. I think this is sufficiently unambitious that adopting such a design would be harmful to Amaranth SoC
<tpw_rules>
and the comment earlier "Memory maps are attached to bus interfaces" suggests to me that jfng[m] thinks that memory maps are a property of instances too. they just have to be given to the type because of how interfaces are created internally
<whitequark[cis]>
I think that they may have misspoken
<jfng[m]>
you shouldn't have to build a peripheral (or any component that has a bus) to obtain its memory map
<whitequark[cis]>
tpw_rules: galibert: consider this: when you change the memory map (and anything downstream, e.g. CSR fields), you have to rebuild your software
<whitequark[cis]>
with the proposed design, we have a clear way to determine if you do or do need to rebuild downstream dependencies: if the signature changes, you do (whether that's HDL or software dependencies); if it does not, you do not, you keep the same SVD files, etc
<whitequark[cis]>
* or do not need to
<tpw_rules>
okay, that's a cogent way to look at it
<tpw_rules>
thank you for explaining
<whitequark[cis]>
if you are releasing a new silicon revision of an existing SoC, you want to ensure the type stays the same
<whitequark[cis]>
or that the new type is a Liskov subtype of the old one
<whitequark[cis]>
(we could build automated checks for register maps for this! that would be really handy)
<tpw_rules>
then let me pull back a bit: can we modify the design of the register definitions so that the memory map feels more like defining a type than an instance thing?
<tpw_rules>
(i think this type vs instance confusion might also be affecting csr.field.RW etc)
<whitequark[cis]>
I don't think it does; how would it?
<whitequark[cis]>
in case of a csr.field, the class declaration is the type, and the class instance is the instance
<whitequark[cis]>
actually, I think a lot of things probably should have their signature be defined as a class attribute
<whitequark[cis]>
oh.
<whitequark[cis]>
yes this is completely wrong, I agree
<whitequark[cis]>
it should be something like rdy : csr.field.R[1] for the annotations
<tpw_rules>
the illustration is wrong or the implementation is wrong?
<whitequark[cis]>
I think the illustration just wouldn't work right now, if I'm not missing anything
<tpw_rules>
it works fine, i'm using it
<whitequark[cis]>
hm
<tpw_rules>
but it means if you make a second UARTPeripheral, then both have the same field (and you get the amaranth DriverConflict warning)
<whitequark[cis]>
yes, this is just completely wrong
<whitequark[cis]>
I misunderstood what you were saying earlier
<tpw_rules>
so i think there's something wrong there where a csr.field.RW is both a type and an instance
<whitequark[cis]>
I thought you were somehow manually adding the same field into two different peripherals
<jfng[m]>
aah
<whitequark[cis]>
this is just a plain bug
<galibert[m]>
jfng: What do you mean by build? Does that mean running elaborate() ? Do you plan to require all decoder connections to be required to be done in the constructor (I wouldn't mind, but you have to detect it and bitch if incorrect)?
<whitequark[cis]>
galibert: there is no fixed set point for when you have to stop updating the signature
<whitequark[cis]>
(or its memory map)
<whitequark[cis]>
rather, right before you do something that depends on the specific values in the signature, you must call sig.freeze()
<galibert[m]>
"you shouldn't have to build a peripheral (or any component that has a bus) to obtain its memory map" is what I'm trying to understand
<whitequark[cis]>
freeze() exists specifically to allow you to defer making connections to the decoder past the constructor (but before elaborate)
<whitequark[cis]>
yes, you should not need to elaborate the design to get the memory map
<whitequark[cis]>
i.e. the abstract design hierarchy (which peripherals connect to which, etc) should be determined by something that is outside elaborate
<whitequark[cis]>
it does not need to be done in the constructors, though it could be
<galibert[m]>
There's nothing between the constructor and elaborate afaik
<whitequark[cis]>
you can call anything you want in your design
<whitequark[cis]>
Amaranth doesn't require that you follow a construction with a call to elaborate right away
<whitequark[cis]>
simple designs, which is the majority of existing ones, do not make use of this freedom, but that doesn't mean it doesn't exist
<galibert[m]>
I never call elaborate explicitely
<jfng[m]>
whitequark[cis]: how about, removing field definitions with variable annotations from the RFC ?
<whitequark[cis]>
you call build or convert or something, which calls elaborate
<galibert[m]>
so I don't call it right away, I don't call it at all :-)
<whitequark[cis]>
please read between the lines, you know exactly what I meant
<tpw_rules>
jfng[m]: i think the illustration is nice and i like it, i just think the implementation has a problem
<galibert[m]>
I guess m.submodule += ... takes care of that, but honestly I'm not even 100% sure
<whitequark[cis]>
no, m.submodule just records that you added one
<tpw_rules>
to use the verbiage earlier, csr.field.RW(x) should define a type. then some other process converts it to an instance
<whitequark[cis]>
actual elaboration does not happen until convert() or build()
<galibert[m]>
So, honestly, I don't know when elaborate is called
<galibert[m]>
especially in submodules
<galibert[m]>
I do know that it's usual (even if I avoid it) to have submodules created in the elaborate of the module using them
<whitequark[cis]>
the specific order of elaboration is intentionally undefined; you should not be mutating the design in elaborate in first place (this wasn't possible in Migen, and is why elaborate() even exists)
<whitequark[cis]>
tpw_rules: yeah. or rather, whatever lives in the annotation should define a type
<galibert[m]>
Is there a design structure before m.submodule += is called?
<whitequark[cis]>
per Python conventions this should be self.field.RW[x], but the problem is that self.field.RW[x, reset=1] is not valid syntax
<galibert[m]>
It's unclear to me
<tpw_rules>
whitequark[cis]: didn't you bump into that before? or was that the same problem, just earlier?
<whitequark[cis]>
galibert[m]: tpw_rules: the same problem in `lib.wiring`
<jfng[m]>
yeah, iirc we considered type annotations early in the RFC
<whitequark[cis]>
jfng: I propose a 1-1 session sometime next week where we brainstorm solutions to this
<whitequark[cis]>
I don't think we need to fundamentally change the RFC or drop annotation support
<whitequark[cis]>
@tpw_rules: I think that's the only viable path, in fact
<whitequark[cis]>
if you do not, you will keep adding stuff to the memory map of that one signature you created on line 60
<tpw_rules>
what do you mean by "keep adding"?
<cr1901>
jfng[m]: Even if it doesn't fit, I'll prob keep the CSR regs around in a branch and then merge into main once the PR's merged. I have have other options to reduce size.
<whitequark[cis]>
well, there is exactly one instance of csr.Signature shared by every SampleWriter object
<tpw_rules>
yes
<whitequark[cis]>
this is why your approach of using __annotations__ is illegal
<tpw_rules>
for sure it is
<whitequark[cis]>
there is a weird thing going on with some signatures and elaboratables that muddies the type analogy somewhat. not really if you're into complex type systems, but definitely if you only know like, C
<whitequark[cis]>
where some elaboratables have one type for each instance of the Python class, but some elaboratables have a type per instance of the Python class
<tpw_rules>
isn't that the same thing
<whitequark[cis]>
er
<galibert[m]>
Catherine: There's something I keep returning to in my mind, is that you can't say "to connect to this initiator, say a wishbone bus one, you need to be of this type", because "this type" doesn't exist as a unique thing, you have potentially as many types as you have objects connected to the bus
<whitequark[cis]>
s/each/every/
<galibert[m]>
if type == signature
<tpw_rules>
okay. then right now it sounds like a memory map locks you into the second category
<whitequark[cis]>
that's correct
<whitequark[cis]>
well
<whitequark[cis]>
actually, no, it's not
<whitequark[cis]>
the memory map of a register where the fields aren't parametric could be created once and frozen
<whitequark[cis]>
... oh, this also answers the question of what to do for the issue you've filed earlier
<whitequark[cis]>
the signature in Component (when it's created from the annotations) should be created once in __init_subclass__ (!), frozen, and assigned as a class attribute
<tpw_rules>
so that definitely dooms you to not use Component if you have a csr.Signature
<whitequark[cis]>
not to use Component annotations
<tpw_rules>
what is Component without its annotations?
<whitequark[cis]>
you can always define a @property def signature()
<tpw_rules>
okay, so that would be the sanctioned way
<galibert[m]>
An Elaboratable
<whitequark[cis]>
tpw_rules: there is also the metadata we're working on
<whitequark[cis]>
I think inheriting from wiring.Component is also valuable because it communicates intent
<whitequark[cis]>
teechnically you do not need to inherit from Elaboratable either
<whitequark[cis]>
(yes, it shows a warning, but that's only because people forget adding submodules so often; it is mostly just a marker)
<tpw_rules>
would it be possible/sane for csr.Signature to accept a list of class objects which inherit from csr.Register and then build the memory map internally?
<tpw_rules>
(idk if the scoping works out for that)
<whitequark[cis]>
("this is an Amaranth thing that becomes a part of the netlist")
<whitequark[cis]>
tpw_rules: that doesn't feel like the right approach
<tpw_rules>
then possibly "best" would be to have the signature property grab super().signature, then create a new one with csr_bus on it
<whitequark[cis]>
I dunno what is better, we need to rethink this whole approach and I'm tired
<tpw_rules>
okay. i'd like to come back to it in the future
<whitequark[cis]>
yeah
<whitequark[cis]>
anyway, the meeting ran a hour over the allotted time, it is concluded now
<tpw_rules>
maybe you and jfng[m] can fix the CSR bug then we three can put our heads together again later
<whitequark[cis]>
jfng, when you have time, could you please summarize the conversations?
<whitequark[cis]>
tpw_rules: it's a design bug, it cannot be fixed without, well, redesign
<tpw_rules>
iiuc it could be fixed without user-visible changes
<whitequark[cis]>
nope
<tpw_rules>
(whether it should be is a different matter
<tpw_rules>
)
<whitequark[cis]>
though, hm
<whitequark[cis]>
no, I don't think it can be
<whitequark[cis]>
a Field is a Component and you wouldn't be able to make an array of registers unless it becomes something else or stops being created like this
<whitequark[cis]>
(I don't consider deepcopy a serious option)
<tpw_rules>
my solution would be to make Field not a Component and have it be some pre-component like a Signature vs. an Interface
<whitequark[cis]>
that's a user-visible change...
<whitequark[cis]>
we also did quite want to have each Field be responsible for encapsulating its storage
<whitequark[cis]>
so the Field would have to be split from FieldType or whatever
<tpw_rules>
i don't think it changes the text of the RFC
<tpw_rules>
for sure
<whitequark[cis]>
it definitely changes the text of the RFC because the new entity has to appear in it
<tpw_rules>
anyway. please rest, this can be adjusted later
<whitequark[cis]>
oh, I'm not going to rest, I have plenty of things to do today
<whitequark[cis]>
(which is why I'm so disappointed about this meeting running a hour too long)
<jfng[m]>
<whitequark[cis]> "jfng, when you have time..." <- yes
<tpw_rules>
apologies
<whitequark[cis]>
but yes, I'm glad we had this meeting, it was very illustrative
<whitequark[cis]>
galibert: after some preliminary consideration, I think we may yet have to go the way you propose, and give up on going this far with the "type" analogy
<whitequark[cis]>
mainly because the terminology and the semantics may just become too difficult to reasonably teach
<galibert[m]>
It confuses me immensely, that’s for sure
<tpw_rules>
i see the value in it now, i'm not sure if i'd want to grok it constantly
<whitequark[cis]>
to both of you: yes, this is the main consideration I have
<whitequark[cis]>
I think it might be more practical to redefine "metadata" as "neither type nor concrete netlist but something in between"
<tpw_rules>
that's where my money is
<whitequark[cis]>
galibert: if we do this, the memory map will live on the interface object, which is attached to the instance
<whitequark[cis]>
so you'll get the behavior you proposed
<whitequark[cis]>
we still need to figure out something about the field
<whitequark[cis]>
I think what should probably happen is making csr.Field a marker object like In (not a component), where the actual RW object is an elaboratable
<whitequark[cis]>
then you would do: x : csr.Field(csr.field.RW, ...)
<whitequark[cis]>
then we can keep using keyword arguments in the annotation
<whitequark[cis]>
a Python-compatible manner would be something like csr.field.RW[csr.Field(1, reset=1)]
<whitequark[cis]>
that reflects the fact that self.x would be an instance of csr.field.RW configured using csr.Field(...)
<whitequark[cis]>
but that's kind of ugly and we can support both options anyways
<galibert[m]>
Don’t we have everything else using parentheses only?
<_whitenotifier-b>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] f0a7df0 - Deploying to main from @ amaranth-lang/amaranth@193fdaccd0031881de0cc7034adea33dbb46336a 🚀
ldcd[m] has joined #amaranth-lang
<ldcd[m]>
Silly question but what is the canonical way to rename the clk and reset signals in the generated rtl
<ldcd[m]>
for some reason using DomainRenamer at the end there resulted in a clock that didn't get added as a top level port?
<ldcd[m]>
Ok my other (cursed) question is the memory I'm instantiating is ~2MB and yosys takes about 2 minutes to generate the verilog (I'm assuming because of the massive init list), I don't really care what it is initialized to, can ya'll think of any way to speed that up