<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 12eb174 - Deploying to main from @ amaranth-lang/amaranth@11ec35d258b059f5c8b1c3d4a4fe1538d2bee046 🚀
<Chips4MakersakaS>
I am looking a Minimig (Amiga emulation) code for MiSTer. They are using PLLs and custom interface to the on-chip ARM cores as Quartus IP. AFAICS there is no common way in handling these Altera IP in Amaranth and you need to manuallly `Instance()` them yourself. Correct ?
<galibert[m]>
Correct
<whitequark[cis]>
pretty much
<galibert[m]>
Note that it works very well with an instance
<whitequark[cis]>
interfacing altpll is pretty easy
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] whitequark 934ce08 - Deploying to main from @ amaranth-lang/amaranth@0564dd5d7853bd768aef8cc1bd8405ec0b7e48c5 🚀
<zyp[m]>
<whitequark[cis]> "zyp: that's basically it. the..." <- how about instead of a decorator, we have a `.ensure_testbench()` method on the context that raises if it's not in a testbench process? it saves needing a module level name for the decorator, and it also means that the decorator doesn't have to find the `sim` argument and the restrictions that imposes on decorated functions
<whitequark[cis]>
zyp: I was thinking that that's how the decorator would be implemented
<zyp[m]>
I just don't think having a decorator adds anything, it's one line either way
<whitequark[cis]>
I think a decorator is more idiomatic in Python in this context
<whitequark[cis]>
it annotates the whole function with a different semantic
<whitequark[cis]>
meanwhile, a function call assigning a semantic to the whole function is quite magical
<whitequark[cis]>
think of how a documentation tool would understand that a certain method is a simulation helper
<whitequark[cis]>
parse the code and hunt for a function call?
<zyp[m]>
fair point
<zyp[m]>
how would you have the wrapper functions handle arguments? def wrapper(sim, *args, **kwargs): … f(sim, *args, **kwargs) ?
<whitequark[cis]>
zyp: yes but... I don't think Python requires the self argument to be named `self`.
<whitequark[cis]>
imagine if the function is a testbench helper AND a class method
<zyp[m]>
we could check if it's a unbound/class method and pass the first argument through in that case, otherwise expect sim to be the first argument
<whitequark[cis]>
it's a normal function when the decorator sees it though
<Wanda[cis]>
you cannot know if it's a method because it's not assigned to a class yet
<whitequark[cis]>
(do you know how decorators / class dictionaries work?)
<whitequark[cis]>
basically, it's accessing something on a class thru an object that imbues it with methodness
<Wanda[cis]>
you could check if you're in a class scope by crimes, but then you won't know if you're in something that will soon be passed to a staticmethod or not
<zyp[m]>
I'm familiar with decorators, just not magically making them work on both methods and functions
<whitequark[cis]>
I think the solution is just to have def wrapper(*args, **kwargs): and then check if args[0] or args[1] is a simulator context
<Wanda[cis]>
thinks
<whitequark[cis]>
and I guess mandate that the first argument or two are positional only
<whitequark[cis]>
which is ... pretty weird
<Wanda[cis]>
why do you need them to be positional only?
<whitequark[cis]>
cause otherwise, if someone passes sim as a kwarg the decorator will not see it
<_whitenotifier-6>
[amaranth] github-merge-queue[bot] created branch gh-readonly-queue/main/pr-1206-11ec35d258b059f5c8b1c3d4a4fe1538d2bee046 - https://github.com/amaranth-lang/amaranth
<Wanda[cis]>
oh right, it's not called by our simulator, it's a helper
<Wanda[cis]>
uh
<Wanda[cis]>
<Wanda[cis]> "do not ask catgirl complex..." <- yeah you may want to refer to that
<Wanda[cis]>
ok, but also
<Wanda[cis]>
if the wrapper sees a kwarg named sim, we can just ... look at it?
<Wanda[cis]>
ok no, you're right, positional-easy is just less messy
<whitequark[cis]>
but there's no actual requirement to name it sim
<zyp[m]>
unless we make it one
<whitequark[cis]>
the same problem will later manifest itself with a "this is a function over Module" decorator
<whitequark[cis]>
where the body must be executed in the root scope of the Module and not the inner one, to respect lexical nesting of with m.If
<Wanda[cis]>
the wha... oh
<Wanda[cis]>
you want something like that
<Wanda[cis]>
hm
<whitequark[cis]>
people keep constructing those things
<Wanda[cis]>
true
<Wanda[cis]>
so the easy ways out of it are 1) separate decorators (or a decorator arg) for plain functions and methods, 2) just look at both args[0] and args[1]
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 4e8c9f2 - Deploying to main from @ amaranth-lang/amaranth@83701d74cfca7d684f1137a3eb5d277d1ef572ce 🚀
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] whitequark 39f07c9 - Deploying to main from @ amaranth-lang/playground@8c80da5fc53432a2dc30fea72b8c1a5009c93685 🚀
<jfng[m]>
there is a backward-compatibility question for a register with a single anonymous field, in the scenario where you want to add another one in the future
<jfng[m]>
then the existing field would need to have a name
<jfng[m]>
although this constitutes a breaking change anyway, for other reasons (e.g. changing the size of the register)
<jfng[m]>
so this is probably not a concern
<tpw_rules>
yeah, it could really confuse the code but that's unlikely, and should be localized
<whitequark[cis]>
I'm on a train so my connectivity is bad
<whitequark[cis]>
I do have comments though
<whitequark[cis]>
if we're changing .fields to .field, the constructor argument should be changed too
<whitequark[cis]>
which would be pretty weird
<cr1901>
I thought it would be changed to .field for only single-field regs
<jfng[m]>
cr1901: that's somehow worse
<tpw_rules>
i don't think it's that unjustified to have the property and the constructor be different. "fields=" is the fields on the register, ".field" is to access a specific one
<whitequark[cis]>
is `fields=Field(...)` not problematic?
<tpw_rules>
a) when would you do that? b) it's still pointing to the fields that will be on the register, it's just one field this time. i guess you could say that implies a data structure
<jfng[m]>
also `fields=` and `.fields` are not the same (type, etc), so i don't think we have a strong incentive to make them have the same name
<tpw_rules>
but `field=[Field(...)]` is still weird in the same way
<whitequark[cis]>
tpw_rules: no, the list is plural
<tpw_rules>
so we would have two constructor arguments?
<whitequark[cis]>
that is not weird at all
<jfng[m]>
whitequark[cis]: as i said in the GH thread, for a single-field register, you would use something like `super().__init__(csr.Field(...), access="rw")`
<whitequark[cis]>
jfng[m]: ok, good point
<jfng[m]>
not the kwarg form
<whitequark[cis]>
I'm ok with fields= + .field
<tpw_rules>
i am too
<whitequark[cis]>
jfng[m]: don't have email on my phone
<cr1901>
I think .field is weird if it's for multi-field regs
<jfng[m]>
in both cases, you'd use `.f` 99% of the time
<jfng[m]>
which is an alias to `.field`
<tpw_rules>
you also are trying to access only one field through .field
<tpw_rules>
at a time
<cr1901>
that is true...
<Chips4MakersakaS>
Rather than doing it in Register maybe a SingleFieldRegister could be made for this functionality ? This would then have both .field and .fields
<Chips4MakersakaS>
SingleFieldRegister subclass of Register
<cr1901>
I kinda like that more
<jfng[m]>
i think that would more complexity for a relatively basic use-case, the CSR register API already has a lot of moving parts
<tpw_rules>
jfng[m]: that is my feeling, i don't want to be introducing more types
peeps[zen] has quit [Remote host closed the connection]
peeps[zen] has joined #amaranth-lang
<cr1901>
.field is growing on me the more I think about it
<whitequark[cis]>
yeah I don't want to see more types either
<cr1901>
maybe if there's a pressing reason to access all fields at once, .fields can be reintroduced in a backward-compat (sic, since we're getting rid of it) manner with .field
<tpw_rules>
well .field would still work for accessing multiple fields, it's just the primary case is one field
<cr1901>
Yea .field is fine
<jfng[m]>
unless someone has further comments, let's initiate a vote
<whitequark[cis]>
Lunaphied[m]: I'm working on a mini-streams RFC
<whitequark[cis]>
I plan to publish next week
<Lunaphied[m]>
Ah okay, thank you!
<tpw_rules>
whitequark[cis]: okay, so the next step is to revise the RFC based on feedback?
<whitequark[cis]>
tpw_rules: yes
<tpw_rules>
then once it gets merged, i create a tracking issue and the implementation PR?
<whitequark[cis]>
you can also rename the file, add the date, etc, though if you don't I will
<whitequark[cis]>
tpw_rules: yep
<whitequark[cis]>
we should write down the procedure
<tpw_rules>
what date is there to add?
<whitequark[cis]>
Start Date in the header
<whitequark[cis]>
(should be today'sl
<tpw_rules>
it's already there?
<whitequark[cis]>
* (should be today's)
<tpw_rules>
okay. i also speculatively filled in the number and name like some other RFCs
<whitequark[cis]>
that's the wrong date -- you did not read the instructions
<whitequark[cis]>
start date is the date of decision to merge, not the date of when you started writing the RFC or submitted the RFC PR draft
<tpw_rules>
okay, where does it say that?
<whitequark[cis]>
a lot of people are getting this wrong though
<zyp[m]>
«start» being «acceptance» is not very obvious
<tpw_rules>
yeah, the only docs i see are "today's date"
<whitequark[cis]>
tpw_rules: in the RFC repo README
<whitequark[cis]>
(and on the /rfcs/ page)
<tpw_rules>
i don't see the word "date" or "start" anywhere on that page. i really don't think it's there
<whitequark[cis]>
oh, huh.
<whitequark[cis]>
okay that would explain why
<whitequark[cis]>
you're right, absolutely nothing says that. I could swear something did!
<whitequark[cis]>
sorry
<whitequark[cis]>
I should fix that to make it clear, and also write down the process for what to do when the RFC is accepted
<tpw_rules>
it would be useful
<Wanda[cis]>
yeah I'm pretty sure you only told me that in a DM
<Wanda[cis]>
(and I didn't double-check the docs)
<whitequark[cis]>
oops.
<jfng[m]>
@libera_tpw_rules:catircservices.org just realized RFC 57 should also work with an anonymous field array (e.g. `csr.Register([csr.Field(...) for _ in range (n)])`)
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] whitequark 3ed42ff - Deploying to main from @ amaranth-lang/rfcs@2f47e702c0dbabab2703c4c71eebb1c79949e3c6 🚀
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] whitequark d5abeba - Deploying to main from @ amaranth-lang/rfcs@e84cc621d3c073aebe4b7c778e7880b655e8d202 🚀
<_whitenotifier-6>
[amaranth-lang/rfcs] whitequark 885b276 - Write down the procedure for merging an RFC.
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] whitequark de5f032 - Deploying to main from @ amaranth-lang/rfcs@885b2766ba557c1634236c1198875c9d2486078f 🚀
<_whitenotifier-6>
[amaranth-lang/amaranth-lang.github.io] whitequark 603480b - Deploying to main from @ amaranth-lang/rfcs@699bf4d8f00ec5a4e3f948aa010afc034e9f73ac 🚀
<zyp[m]>
Catherine: which exceptions should `testbench_helper` raise? I figure `TypeError` if it can't find the simulation context, but what if the process type is wrong? `AssertionError`? `RuntimeError`? a custom exception?
<whitequark[cis]>
I think that can be left unspecified since we don't really expect anyone to catch it
<whitequark[cis]>
(unlike e.g. return values of awaitables)
<zyp[m]>
true
nates93[m] has quit [Quit: Idle timeout reached: 172800s]
<polysci_00232[m]>
Just got around to bumping my project to the lastest commit, seems like FSMs no longer have a 'state' attribute. Is there any replacement or is that feature no longer supported?
<Wanda[cis]>
that feature turned out to be ... problematic
<Wanda[cis]>
it conflicted with making AST nodes immutable (because the shape of state cannot be finalized until the FSM is done)
<Wanda[cis]>
may I ask what your usecase is?
<Wanda[cis]>
(state can still be accessed I think, but only once the FSM is closed)
<polysci_00232[m]>
purely for debugging FSMs
<polysci_00232[m]>
ok sounds like I just have to change how I was accessing it then