_whitenotifier-3 has quit [Ping timeout: 245 seconds]
<whitequark[cis]>
<crzwdjk> "It's not inevitable that a CPU..." <- from the point of view of a HDL, which has to support any ROM, it kind of is; you can't reasonably fail if someone doesn't use the exact boot sequence that initializes all memory
<whitequark[cis]>
<cr1901> "It's yosys... I found past-me'..." <- I think you might just be observing the ice40 bram erratum?
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] whitequark be661c5 - Deploying to main from @ amaranth-lang/rfcs@d7337b131c4c629922a975dc3955bb5920d3d8dd 🚀
<_whitenotifier-7>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] aa708a7 - Deploying to main from @ amaranth-lang/amaranth@089213e19fa66012c68f0445b213b8b321d40a18 🚀
<_whitenotifier-5>
[amaranth] github-merge-queue[bot] created branch gh-readonly-queue/main/pr-1063-089213e19fa66012c68f0445b213b8b321d40a18 - https://github.com/amaranth-lang/amaranth
<Chips4MakersakaS>
<whitequark[cis]> "in fact, it ensures that..." <- What I don't like about it is that you may give impression to current users that using `init=` is something supported for all memories but that it actually is not supported on most implementations if you have a write port on the RAM.
<_whitenotifier-7>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] bcc45ce - Deploying to main from @ amaranth-lang/amaranth@5e2f3b7992632770adfa3ad8c2cfe53ff9cd39fe 🚀
<Chips4MakersakaS>
Problem is that I don't see how you can support giving actual init value for ASIC SRAMs; by now enforcing init= parameter people may falsely think it is a general supported construct for memories.
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): this RFC actually improves the situation for ASIC SRAMs, by creating a way for removing `init=` in the future
<whitequark[cis]>
before the RFC, Amaranth implicitly initialized all memories (to zero, if not otherwise specified), so there wasn't a way to opt out of that
<whitequark[cis]>
now a future RFC can make init= optional, making ASIC SRAMs a possible inference target
<galibert[m]>
Could be init=False actually now that I've slept on it
<whitequark[cis]>
galibert: the RFC suggests `init=None`, which is more or less the same idea
<galibert[m]>
can you distinguish init=None from no init parameter given?
<galibert[m]>
you hide a special default value I guess?
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): my reasoning is that making the current situation (everything is always initialized) explicit is a first step towards changing it (making things sometimes uninitialized)
<whitequark[cis]>
galibert: one simple option is to continue requiring `init=`, but accept either `init=None` or `init=some_data`
<whitequark[cis]>
then you don't have to do the special default value trick (you can also use **kwargs instead of that trick, but these are both kind of annoying for writing wrappers)
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): also, the RFC allows you to make a Memory subclass (maybe ASICMemory or UninitMemory) which simply ignores this parameter, even today. it wouldn't be a core Amaranth thing but it will solve the problem in the immediate term
<galibert[m]>
oh yeah, it's not lua, not giving a parameter gives an error, not sets it to nil (aka None)
<Chips4MakersakaS>
I would prefer supporting init=None in this RFC and even give warning on creaitng a write port on a memory that has explicit memory initialization.
<galibert[m]>
chips4makers: hey, leave my writable bootstrap code in my fpga alone :-)
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): FPGAs support initialized memories with write ports
<galibert[m]>
(otoh I don't use Memory because quartus sucks)
<whitequark[cis]>
galibert: huh, it breaks on quartus? how?
<galibert[m]>
it doesn't break, but the synth time is quadratic with the size
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): the way this works is that the write port is "borrowed" during bitstream loading to set the contents, but is free for use afterwards
<whitequark[cis]>
galibert: ohhhhhh, *that* bug
<whitequark[cis]>
yeah, quartus is ... quartus
<galibert[m]>
so as soon as it's big I Instance m10ks and do the decoding by hand, which is absolutely trivial with amaranth
<whitequark[cis]>
galibert: you could write code for amaranth that uses this RFC to instance m10ks in a platform method now
<galibert[m]>
that would be nice
<Chips4MakersakaS>
whitequark[cis]: I understand but as you said it is more the exceptional case and code using that feature will not work on ASIC and limit RAM options on FPGA. I would like that this is already clear to new Amaranth users after this RFC.
<Chips4MakersakaS>
Could be (reference) documentation for now
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): I have absolutely no intent to limit the use of that feature, as when you do want it, it's irreplaceable (you can't do it without using a lot of extra resources)
<whitequark[cis]>
so the combination of init= and write ports is here to stay
<Chips4MakersakaS>
How will you support that on ASIC ?
<whitequark[cis]>
I won't?
<Chips4MakersakaS>
So when will people find out that their fancy Amaranth code is not ASIC compatible ?
<whitequark[cis]>
Amaranth in general does not stop you from requesting memories that make no sense, e.g. three write ports. despite the fact that no platform in use by Amaranth supports it, it's legal in case yours in particular somehow does and you want to use that
<galibert[m]>
an extra "allow_write_with_init=True" flag to say that you know what you're doing?
<whitequark[cis]>
whitequark[cis]: we have an open issue for a linter that will suggest that this makes no sense, but even that is advisory, because e.g. Yosys has code to detect that the use of these ports is mutually exclusive and they can be merged
<whitequark[cis]>
galibert[m]: nah. combining write ports with initialization is just one type of memory feature combination you might request, and by far not the weirdest or least supported one (in fact, most FPGA memories support it)
<galibert[m]>
Because it's true that wanting init + write is rare (and often stopgap-y)
<whitequark[cis]>
there's nothing in particular about that combination that requires a new flag
<galibert[m]>
I wanted it because I hadn't yet hooked up the external sram
<zyp[m]>
why would init+write be rare?
<galibert[m]>
but it potentially kills reset ability
<whitequark[cis]>
galibert[m]: that's specific to your use case; plenty of FPGAs have more SRAM than you can do anything with (just look at any US or US+)
<zyp[m]>
if you've got a mini-firmware, it makes perfect sense for code+data to share a single blockram
<whitequark[cis]>
another case where it makes a lot of sense to combine init and write is the case of a configuration RAM or a microcode, where you know what it's going to be in most cases, but still want to be able to overwrite it
<whitequark[cis]>
the fact that ASICs do not offer this feature is completely irrelevant if you never aim to deploy to an ASIC in first place
<Chips4MakersakaS>
zyp[m]: IMO, people should be aware that using the feature severely limits the option of the platforms that memory can be implemented, e.g. specific RAM options on certain FPGAs.
<whitequark[cis]>
Chips4Makers (aka Staf Verhaegen): init+write works for almost all RAMs on almost all FPGAs; iCE40 SPRAM is actually the only counterexample I know, though there are probably more
<whitequark[cis]>
RAMs on ECP5, iCE40 (excluding SPRAM), Nexus, Xilinx, Altera, and Gowin are initializable
<whitequark[cis]>
so "severely limits" would be bordering on a false statement
<whitequark[cis]>
it basically does not limit you at all if you only care about FPGAs and your design doesn't fit into iCE40
<Chips4MakersakaS>
As said I don't want to feature to be gone but I want people to know the implication of using it before they actually try to implement their code on an ASIC.
<whitequark[cis]>
* fit into the one of the smallest iCE40, * iCE40 chips
<Chips4MakersakaS>
whitequark[cis]: Sorry, my main focus is ASIC so I am tainted.
<whitequark[cis]>
also, I expect that the majority of use of Amaranth will continue to be FPGAs, as most people can't afford an ASIC and will probably continue to not be able to afford an ASIC
<whitequark[cis]>
ah ok, so I looked it up and Xilinx UltraRAM also can't be initialized (for a similar reason as iCE40 SPRAM). though that also doesn't touch relatively speaking that many people, considering that only the highest end Xilinx chips have any URAM at all
<whitequark[cis]>
basically the two cases where you have an FPGA and need uninit memories are: your FPGA is so big costs hundreds of dollars, or your FPGA is so small it can fit only the most basic designs. this is today's reality that can change, but right now it is that
<whitequark[cis]>
* so big it costs hundreds
<whitequark[cis]>
this isn't a reason to dismiss them from the language entirely as the expressive power is important, but most people (by numbers) who learn about memories will probably never touch either
<whitequark[cis]>
and we need some kind of table summarizing what memories support what features
<Chips4MakersakaS>
whitequark[cis]: Having that in the documentation would indeed fulfill what I am asking.
<whitequark[cis]>
the main issue with that is that FPGAs are very diverse and it's actually quite difficult to make sure that this information is correct and remains up-to-date
<whitequark[cis]>
I don't know who will spend the resources to install something like 7 or 8 FPGA toolchains and run many code examples on each to make sure that, yes, this configuration is supported or not supported (and will keep doing that every year or two)
<whitequark[cis]>
I certainly don't have that kind of time on my hands
<Chips4MakersakaS>
<whitequark[cis]> "yes, which is why the Amaranth..." <- Something else; problem that makes X-propogation in simulation difficult is the so-called X-poisoning; e.g. simulator not getting out of a state where a state register stays X.
<Chips4MakersakaS>
Give for example the JTAG state machine. For JTAG `TRST` is optional and state machine is guaranteed to go to `Test-Logic-Reset` state after having 4 `TMS=1` in a row. If you want to simulate that you actually need to keep track of state space reduction with each `TMS=1` in order to end up in `Test-Logic-Reset`. I was surprised to see free ModelSim seemed to actually do that on a FSM generated from nMigen `FSM` code.
<whitequark[cis]>
yeah. if we implement X-propagation I think we should also implement randomization of indeterminate values as an alternative strategy
<whitequark[cis]>
this is a part of why I don't know if we even should implement X-prop
<Chips4MakersakaS>
agree
<galibert[m]>
Not sure I understand the jtag thing. What is detecting the four tms cycles and resetting the state?
<whitequark[cis]>
the JTAG FSM
<galibert[m]>
Hard IP or the amaranth code?
<galibert[m]>
So it’s supposed to write the state and the x go away, no?
<whitequark[cis]>
it's designed so that no matter what state it's in, toggling TCK with TMS=1 five times puts the FSM into Test-Logic-Reset
<whitequark[cis]>
yes, but if you do an operation on X you get X
<galibert[m]>
Which x would it be that sticks, the counter?
<Chips4MakersakaS>
The state register of the FSM
<galibert[m]>
Ah, the counting is integrated in the state register?
<Chips4MakersakaS>
The state machine for JTAG is documented in the JTAG SPEC, state transistions in the FSM are based on TMS=0 and TMS=1 and are guaranteed to converge to reset state after 5 TMS=1 transistions starting from any state in the FSM.
<galibert[m]>
Oh, I didn’t know there was a standard jtag fsm
<whitequark[cis]>
um... how would JTAG work otherwise?
<galibert[m]>
You could have a description of the expected behaviour and everyone does their thing
<whitequark[cis]>
that's what the standard does. the behavior is described with a state machine
<whitequark[cis]>
(you also have a specific implementation in specific logic gates, but I don't think anybody cares anymore)
<whitequark[cis]>
jfng: observation: when you export a component with e.g. `verilog.convert`, the `reset` value for its inputs is simply lost
<whitequark[cis]>
so metadata is actually necessary for preservation of correctness in cases like that
<Wanda[cis]>
<whitequark[cis]> "ah ok, so I looked it up and..." <- the US+ UltraRAM is actually a secret third thing: it *is* initialized, but always to all-0
<Wanda[cis]>
ie. it is already supported by amaranth, as long as init = [0, 0, 0, ...]
<Wanda[cis]>
(and Versal UltraRAM actually supports initialization just like BRAM, but who can afford that)
<whitequark[cis]>
huh
<Wanda[cis]>
so this is probably just me and my shitty firefox URL autocompletion, but
<Wanda[cis]>
any chance we could get a link to /rfcs/ on amaranth-lang.org landing page? pretty much always when I end up there it's because I was typing the RFC book URL and hit enter too soon
<galibert[m]>
mumble bookmark toolbar mumble
<Wanda[cis]>
... yeaaah I should probably start using that
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] whitequark 7f87636 - Add link to RFCs to landing page
<Wanda[cis]>
thanks :3
jojo404[m]1 has joined #amaranth-lang
<jojo404[m]1>
"I'll help anyone interested on how to earn 100k in just 24hours from the crypto market. But you will have to pay me my commission! when you receive your profit! if interested click on the group link and send me a direct message
<jojo404[m]1>
by asking me HOW
<jojo404[m]1>
@EDWARDJAMES_0
<galibert[m]>
Cleanup on aisle three
jojo404[m]1 has left #amaranth-lang [#amaranth-lang]
<galibert[m]>
:-)
<whitequark[cis]>
zyp: I'm thinking about the interactions of simulation helper functions with domain renaming
<whitequark[cis]>
morally, you want yield Tick('sync') within e.g. a FIFO to use the same "sync" name as for the FIFO itself, without any additional work
<zyp[m]>
yeah, it wouldn't be a problem if we already had a way to associate clock domains with interfaces
<whitequark[cis]>
the way this could work is by wrapping such functions into some decorator like @simulation_helper, keeping track of which domains are renamed into which during elaboration (when this happens within the simulator), and having the simulator run each simulation helper in the appropriate environment
<whitequark[cis]>
oh, right, the helper function would be on the interface object, not on the elaboratable
<zyp[m]>
yes
<whitequark[cis]>
right. I think for now we should probably take a domain argument in these (which is annoying, I know) and then, once we do have this association, use that
<whitequark[cis]>
(though, even if we had something like Out(1, domain="sync"), you still need to associate that "sync" with a particular scope, right?)
<galibert[m]>
I would see domains as parameters of the full components, not of individual sub-interfaces/signals
<whitequark[cis]>
then you can't make an interface which uses two clock domains, or even just a signal with comb feedback
<Wanda[cis]>
... other interesting question
<galibert[m]>
errr, why?
<Wanda[cis]>
should Tick respect any EnableInserters you end up inside of?
<whitequark[cis]>
which limits how much you can use composition
<zyp[m]>
galibert[m]: an `AsyncFIFO` would have two stream interfaces that are identical apart from the direction and the clock domain
<whitequark[cis]>
Wanda: huh, yeah.
<galibert[m]>
ok, so a signal could be a parameter anywhere in the interface tree
<galibert[m]>
s/signal/domain/
<whitequark[cis]>
I'm wondering if we should have like... clock domain members, just like we have port members and interface members
<galibert[m]>
the main difficulty is not making single-domain-sync more verbose than necessary
<zyp[m]>
I figure having clock domain being part of an interface would serve two purposes; it'd allow connect() to verify that you're not trying to connect interfaces being driven by incompatible domains, and it'd allow propagating the domain to stuff like simulation helpers and io registers
<zyp[m]>
I'm not sure how feasible general propagation would be, but it'd be neat if we could e.g. instance an AsyncFIFO without DomainRenamer and instead have the domains be propagated from the interfaces it gets connected to
<whitequark[cis]>
it shouldn't be "propagation" but rather direct and explicit connection
<whitequark[cis]>
zyp: I don't think your rebased implementation is correct any more after RFC changes
<zyp[m]>
indeed, it's from before you removed the domain argument
<whitequark[cis]>
there's also an additional warning
<_whitenotifier-5>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 10a726f - Deploying to main from @ amaranth-lang/amaranth@15b6068c57d7d6bfd7d80e99560595418230a4dc 🚀
<whitequark[cis]>
(also, I have a WIP branch that I should probably just push and leave to you)
<whitequark[cis]>
actually, the sim-bench-processes branch in amaranth-lang/amaranth is now updated
<whitequark[cis]>
I have 2h of meetings now, so feel free to start from that
<Wanda[cis]>
I am starting from zyp's code actually
<Wanda[cis]>
which is probably just fixed up version of youir code
<whitequark[cis]>
hm, you should probably start with mine then
<Wanda[cis]>
fair
<whitequark[cis]>
because I've done significant changes to the logic there
<Wanda[cis]>
okay, yeah, I see
<Wanda[cis]>
okay, so AFAICS the main bulk of work is updating the testsuite
<whitequark[cis]>
yeah
<Wanda[cis]>
hrm
<Wanda[cis]>
in the wrappers, when yield command throws an exception and we do generator.throw(e), we should actually expect the throw to return another command, no?
<Wanda[cis]>
ugh and throw can throw as well
<Wanda[cis]>
I... dislike writing wrappers over python generators, I think
<Wanda[cis]>
I'm going to write something horrible.
<Wanda[cis]>
hmmmmm.
<Wanda[cis]>
now I find myself wanting a generator.warn that injects the warning into the generator and maybe throws in it
<Wanda[cis]>
except
<Wanda[cis]>
holy shit no.
<Wanda[cis]>
no
<Wanda[cis]>
no
<Wanda[cis]>
aaaaaaaaa
<galibert[m]>
[infinite scream]
<Wanda[cis]>
holy meowing god, this is bad
<Wanda[cis]>
I am in a bad place
<whitequark[cis]>
yep
<Wanda[cis]>
okay
<Wanda[cis]>
I don't care that Python doesn't let you warn into a generator
<Wanda[cis]>
I'm going to do it anyway
<Wanda[cis]>
I'm warning you
<Wanda[cis]>
I'm warning the fuck out of this generator
<Wanda[cis]>
I'm going to get my engineers to invent a combustible warning that burns your house down
<Wanda[cis]>
actually
<Wanda[cis]>
how does the warning filtering machinery itself even interact with generators
<Wanda[cis]>
like, if I put up a warning filter and then yield, is it going to affect random other code that executes until my generator is resumed
<Wanda[cis]>
it's one of these cases where I'm going to learn many things about Python and scream a lot, isn't it
<Wanda[cis]>
<galibert[m]> "but then... it's not a warning..." <- it's a combustible warning that burns your stack frame down. my engineers invented it.
<whitequark[cis]>
I'm so glad I didn't have to write that; I got about 30% in and then got interrupted by a meeting
<whitequark[cis]>
though that vs two hours of meetings in a row is kind of a hard choice
<_whitenotifier-7>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 81a66a6 - Deploying to main from @ amaranth-lang/amaranth@f48b8650c4df42b3260c4791b18fad7991af0541 🚀
<_whitenotifier-7>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] af75f8e - Deploying to main from @ amaranth-lang/amaranth@9e75962c352bc73f4b848acc8d3f5e8c5ed6d02f 🚀
<zyp[m]>
the object.__new__ trick is cute, I would probably just have renamed it to _Settle, made a wrapper called Settle and put the deprecation on the wrapper
<whitequark[cis]>
we need to figure out something about `add_sync_process` tho
<zyp[m]>
oh, I didn't realize until now we also deprecated add_process
<whitequark[cis]>
it doesn't seem to serve any purpose given add_testbench
<whitequark[cis]>
btw, I wonder if we even should asyncify add_sync_process
<whitequark[cis]>
I'm not actually sure if the overall shape of that API is the right one?
<whitequark[cis]>
basically every reasonable function added with add_sync_process should have a while True: around it and yield Passive() at the beginning
<whitequark[cis]>
and there's no async equivalent of plain yield
<whitequark[cis]>
I guess the other option is to deprecate add_sync_process entirely instead of deprecating add_process since the latter is more generic
<zyp[m]>
I would probably support that
<whitequark[cis]>
hm, if you can await on multiple changed events, you should be able to substitute comb processes too
<whitequark[cis]>
whitequark[cis]: this would probably need to be another RFC, with its own motivation section and explanation
<zyp[m]>
yeah, I figure it can either be folded into RFC 36, or another that builds on that
<zyp[m]>
I guess an argument for add_process over add_sync_process is that it's more consistent with add_testbench now not taking a domain argument
<zyp[m]>
anyway, I was about to go to bed -- I'll see if I can bring #990 up to current tomorrow and start thinking about updating RFC 36
<whitequark[cis]>
night!
<whitequark[cis]>
and yes, we can probably fold that into RFC 36