<d1b2>
<edbordin> whitequark: does nmigen do any formal stuff atm? I'm considering packaging up yosys-smtbmc and symbiyosys with pip
zignig has quit [Remote host closed the connection]
<awygle>
it does/can yes
<d1b2>
<edbordin> ok cool. perhaps it's slightly off-topic in this channel but figured it's still kinda relevant to the efforts to make it easy to install all the deps to use nmigen
<d1b2>
<edbordin> basically the main issue I have with yosys-smtbmc is that on windows yosys has rolled its own exe launcher and building that with mingw breaks it. I think it would be better to let the pip/setuptools machinery generate it
<cr1901_modern>
edbordin: I'm the one who added that exe launcher
<cr1901_modern>
It "works on my machine", which is clearly unacceptable, and I would like to help you debug it
<d1b2>
<edbordin> cr1901_modern: to be fair my issue is probably largely caused by not building it with msvc
<cr1901_modern>
And I nominally agree, but when I made that PR Claire was lukewarm about packaging yosys-smtbmc and symbiyosys
<cr1901_modern>
edbordin: What is the output of "whereis python3"?
<cr1901_modern>
And what does the shebang line of yosys-smtbmc-script.py look like?
<d1b2>
<edbordin> ah, does the exe launcher read the shebang?
<cr1901_modern>
yes
<d1b2>
<edbordin> well, I guess probably the first issue is that the build didn't place the .py script under /bin for windows. so when I copied in one from the linux build that was probably incorrectly set up
<cr1901_modern>
It reads the shebang to find the python interpreter and then launches the python interpreter with that script as input
<cr1901_modern>
Did you compile yosys w/ MSVC?
<d1b2>
<edbordin> no, it's all done with mingw64
<d1b2>
<edbordin> I'm guessing I might have confused the makefile a bit with this platform
<cr1901_modern>
In any case, let me see the shebang line before I continue
<cr1901_modern>
and we start discussing multiple threads at once- I don't want that
<cr1901_modern>
I _think_ I know what's wrong
<cr1901_modern>
Err.. let me back up
<d1b2>
<edbordin> yes, this definitely won't work on windows: "#!/usr/bin/env python3"
<cr1901_modern>
Do "make install" in your yosys src directory again. WITHOUT the Linux script.
<cr1901_modern>
I want to see what happens to the shebang line because it will be modified
<d1b2>
<edbordin> will take me a bit to set it up again (these outputs are from CI)
<cr1901_modern>
If you set the PYTHON environment variable as an input to make, just like make PREFIX=/path/to/stuff
<cr1901_modern>
you can forcefully set the shebang line to wherever you like
<d1b2>
<edbordin> part of the problem is that I'm not supplying a python interpreter in this distribution, so ideally I want to tell it to just use python.exe if that's possible
<cr1901_modern>
python.exe by itself might work?
<cr1901_modern>
Lemme try on my end
<cr1901_modern>
No, it doesn't work by itself
<d1b2>
<edbordin> would it normally look something like "#!C:.....\python.exe" ?
<d1b2>
<edbordin> yep, it works when I do that
<cr1901_modern>
If it's the full path to your interpreter, yes
<cr1901_modern>
>so ideally I want to tell it to just use python.exe if that's possible
<cr1901_modern>
But since you said that >>
<cr1901_modern>
I was under the impression you didn't know the path ahead of time
<d1b2>
<edbordin> no, I don't, I thought I'd just check the launcher wasn't completely broken the way I built it
<cr1901_modern>
The makefile snippet that builds the launcher and script will try to autodetect your python interpreter
<d1b2>
<edbordin> I guess I could probably patch the launcher so that python.exe does work
<cr1901_modern>
If that doesn't work, perhaps the build should fail and say "you need to set PYTHON=C:\path\to\interpreter" or something.
<d1b2>
<edbordin> the problem is the interpreter at build time is the one in the CI instance
<cr1901_modern>
And you want to distribute the CI artifacts, presumably
<d1b2>
<edbordin> yes, I have already had to do a couple of...questionable...things to make them relocatable
<cr1901_modern>
So you need both the CI and the user's Python to be available
<cr1901_modern>
Not sure how to proceed here
<cr1901_modern>
(And before anyone complains about msys python, this dance is also required for msvc python)
<d1b2>
<edbordin> I have run into something like this before when trying to move a virtualenv on windows. iirc you generally have to run something to fix up these shebangs
<d1b2>
<edbordin> so the launcher is just functioning as designed I guess
<d1b2>
<edbordin> PYTHON is one workaround that's quick to implement though
<cr1901_modern>
Two of the executables that come with windows python- I forget which, py2/py3?- have some shebang intelligence
<cr1901_modern>
Perhaps the launcher could be modified to cache the python install location?
<d1b2>
<edbordin> OK, PYTHON doesn't seem to stop it trying to run the shebang. I guess I could just bundle a script for windows users that finds the py3.exe launcher
<cr1901_modern>
Nota bene: I don't know if "py3.exe" is the correct binary
<d1b2>
<edbordin> well, I definitely have a py.exe
<cr1901_modern>
>PYTHON doesn't seem to stop it trying to run the shebang.
<cr1901_modern>
I explained this poorly
<cr1901_modern>
PYTHON allows you to overwrite the shebang line
<cr1901_modern>
it does not prevent the launcher from running the shebang line
<cr1901_modern>
And I'm not sure if the launcher should be looking for various python interps on your path in the first place
<d1b2>
<edbordin> true, that might be unexpected behaviour if yosys-smtbmc does or does not work depending on what python env is active
<cr1901_modern>
Note this is how pip works too on Windoze- it will modify the shebang line appropriately
<Falteckz>
Are you trying to run python from inside MSYS or some other Unixesque Windows environment?
<Falteckz>
Because if yes - gosh that gets horrible fast.
<d1b2>
<edbordin> no, I am running these tests from cmd.exe
<cr1901_modern>
If it runs from cmd.exe, it'll prob work fine from MSYS.
<d1b2>
<edbordin> I agree, it's a convenient build environment but I would never want the end users to be forced to use msys to use these tools
<cr1901_modern>
edbordin: I just stole the launcher script from setuptools, put the license inline, and made the minimal changes to make yosys-smtbmc work in cmd.exe :)
<cr1901_modern>
(Good thing the license is compat w/ yosys, otherwise I would've never done this :)...)
<d1b2>
<edbordin> cr1901_modern: yep, it seems to behaving as expected, thanks for helping me understand how it behaves
<cr1901_modern>
Yw, glad I could help. A probably decent idea is to modify the launcher so the errors suck less.
<cr1901_modern>
And give a hint to the user that "the launcher thinks Python is at "X", but it wasn't actually at "X""
Degi has quit [Ping timeout: 256 seconds]
<d1b2>
<edbordin> fwiw #!"C:\windows\py.exe -3" seems to work. unsure if everyone on windows will have that launcher though and as previously discussed it is probably not expected behaviour to have the script run in different interpreters
Degi has joined #nmigen
<cr1901_modern>
I don't have py.exe
<d1b2>
<edbordin> it might have been put there by miniconda on my system
<cr1901_modern>
Ahh I don't have miniconda installed anymore (new install as of Jan 2020)
<d1b2>
<edbordin> I think I'm probably just going to have to bundle a script that fixes up the shebang
<awygle>
whitequark added smtbmc to the yowasp distro recently it seems
<d1b2>
<edbordin> wow, that must have meant an entire wasm python interpreter
<cr1901_modern>
edbordin: Open to suggestions. _I_ would prefer whatever you come up with make its way into yosys proper
<cr1901_modern>
to benefit others
<d1b2>
<edbordin> wait no, you already have python, no need for wasm python
<d1b2>
<edbordin> cr1901_modern: is yosys-smtbmc ever used outside of sby? If not I would be in favour of moving the script into sby and making that all a pip package, but it sounds like claire didn't approve of pip packages
<cr1901_modern>
cr1901_modern: Yes, I have used it by itself many times
<awygle>
whitequark also posted a twitter poll about sby recently, i suspect she has Plans
<awygle>
not to discourage you per se
<d1b2>
<edbordin> right. I'm not in a rush, and whitequark knows these codebases / the community around them much better than me
<cr1901_modern>
I was in the process of rewriting sby to use async instead of select(), because select() doesn't work on FDs on Windoze.
<awygle>
ew really? that's lame
<cr1901_modern>
Unfortunately, I ran into a Python bug, filed a bug report, never heard back
<d1b2>
<edbordin> ah ok, so windows is currently not supported anyway. good to know.
<cr1901_modern>
edbordin: Eeeeh
<d1b2>
<edbordin> sounds like you're working on it though
<awygle>
i'm fairly sure i've run it under cygwin :p
<cr1901_modern>
edbordin: I had to put it on the sidelines about a year ago
<cr1901_modern>
I _did_ have a proof-of-concept working
<d1b2>
<edbordin> along the lines of cygwin... it would probably also work in msys python
<d1b2>
<edbordin> but your PR looks like a much better solution
<cr1901_modern>
>Spawning an instance of "TASKKILL /F" to kill the subprocess, and then awaiting asyncio.subprocess.wait is a workaround.
<cr1901_modern>
Oh I forgot about this :D
<cr1901_modern>
wq loved (absolutely despised) this
<d1b2>
<edbordin> haha, whatever gets the job done. tbh even if it were fixed in cpython you'd probably want the workaround anyway for compatibility
<cr1901_modern>
Anyways, glad I could help... point is- I have made an effort to make things better in the past, but I had to stop working on it for various reasons. And I don't have the time/bandwidth to fix things now.
<cr1901_modern>
Since that PR, Windoze support for sby has broken/fixed at least once.
<cr1901_modern>
And when it works, it's really not quite parity w/ *nix b/c of select behavior (among others IIRC)
<d1b2>
<edbordin> cr1901_modern appreciate the help, hope the initial talk about the python launcher thing didn't come across as too critical or anything
<cr1901_modern>
edbordin: No worries... if it only "works on my machine" then it means what I did could be improved.
<d1b2>
<edbordin> well, I think making it just search for python.exe or py.exe goes against how it's designed to work, so it's not really fair for me to expect that. but there might be a minor bug in the makefile where the .py file doesn't get installed into /bin (but it is also likely I'm not building it right)
<d1b2>
<edbordin> I will have to investigate that later
PyroPeter_ has joined #nmigen
PyroPeter has quit [Ping timeout: 260 seconds]
PyroPeter_ is now known as PyroPeter
electronic_eel_ has joined #nmigen
electronic_eel has quit [Ping timeout: 260 seconds]
<d1b2>
<edbordin> ooh, it's a little dirty but I worked out that python launcher does work with relative paths, so I might get away with just copying the mingw python into my distribution
<d1b2>
<edbordin> (I might rename it so it doesn't pop up unexpectedly in people's PATH)
<cr1901_modern>
Coordinate w/ wq before moving forward
phiren is now known as phire
<d1b2>
<edbordin> I think it should work independent of the python env nmigen runs in, but yes I will do that before going too deep
<d1b2>
<edbordin> thing is, I was already bundling most of mingw python for the embedded interpreter in nextpnr so this is actually not hard
<d1b2>
<edbordin> whether it is a good idea is another matter though 😛
<awygle>
are there any other nmigen devs in this channel who can quickly pull master and try to run the test suite?
<awygle>
i am getting a test failure in test_sim
<lkcl>
awygle: if you have time, can i suggest doing a binary search to narrow down which commit causes the failure?
<awygle>
yeah that's what i'm doing now (or, well, looking for suspicious commits anyway)
<lkcl>
that would help whitequark save some time when she's back/available
<awygle>
but if somebody was around i was hoping they could check if it was just my setup
<lkcl>
the brain-dead method is just to cat "git log > /tmp/f"
<lkcl>
and use binary search "git checkout xxxx"
<lkcl>
even if it "was just your setup" it is still useful to determine which (pair of) commits do and do not work
<lkcl>
rather than trying to hunt through reams and reams of text
<lkcl>
you can run "python3 setup.py test" multiple times and get on with something else whilst it's running
<awygle>
it's definitely my setup
<awygle>
sigh
<lkcl>
you went back to the git revision before "git pull"?
<awygle>
i went back to like, february
<awygle>
and i know i passed the test suite in february
<lkcl>
sigh :)
<_whitenotifier-b>
[nmigen] awygle opened pull request #422: Don't inherit Shape from NamedTuple - https://git.io/JJIDW
<awygle>
let's see if it fails in ci
<sorear>
how is that any easier than "git bisect"
<awygle>
i would have bisected it if i could find a working revision
<_whitenotifier-b>
[nmigen] codecov[bot] commented on pull request #422: Don't inherit Shape from NamedTuple - https://git.io/JJID0
<lkcl>
sorear: i often get horribly confused with git bisect. i use the file, cut 50% of it manually, and can use "undo" buffer on the file if needed. it works :)
<_whitenotifier-b>
[nmigen] codecov[bot] edited a comment on pull request #422: Don't inherit Shape from NamedTuple - https://git.io/JJID0
<awygle>
whelp didn't fail in CI (also CI seems to have gotten a lot quicker, wonder if that's yowasp-related)
chipmuenk has joined #nmigen
chipmuenk has quit [Quit: chipmuenk]
hitomi2504 has joined #nmigen
chipmuenk has joined #nmigen
jeanthom has quit [Ping timeout: 256 seconds]
d1b2 has quit [Remote host closed the connection]
d1b22 has joined #nmigen
d1b22 is now known as d1b2
<d1b2>
<edbordin> cr1901_modern: btw turns out the fomu-toolchain scripts I inherited opted not to run make install, so the lack of yosys-smtbmc-script.py in /bin was totally on me
lkcl_ has joined #nmigen
jeanthom has joined #nmigen
lkcl has quit [Ping timeout: 256 seconds]
Asu has joined #nmigen
<_whitenotifier-b>
[nmigen] jeanthom commented on issue #418: Simulation of Verilog output doesn't match nMigen simulation - https://git.io/JJIpW
<whitequark>
edbordin: yosys-smtbmc is already packaged in yowasp fwiw, sby i will talk about it on the next meeting
<whitequark>
Falteckz: yes, "Coming soon" link indeed
<whitequark>
awygle: CI gets quicker or slower depending on commit frequency in nmigen in yosys currently...
noknok has joined #nmigen
noknok has quit [Client Quit]
jeanthom has quit [Ping timeout: 246 seconds]
<_whitenotifier-b>
[nmigen] whitequark commented on issue #418: Simulation of Verilog output doesn't match nMigen simulation - https://git.io/JJIhY
<_whitenotifier-b>
[nmigen] awygle commented on pull request #422: Don't inherit Shape from NamedTuple - https://git.io/JJIhg
<_whitenotifier-b>
[YoWASP/yosys] whitequark pushed 1 commit to develop [+1/-0/±0] https://git.io/JJIha
<_whitenotifier-b>
[YoWASP/yosys] whitequark 8bc590e - [skip ci] Add an auto-updater.
<awygle>
whitequark: I don't understand what you mean by "for equality not for ordering"
<whitequark>
awygle: welll, you can compare tuples like (x,y) == (z,t)
<whitequark>
but you can *also* compare them like (x,y) < (z,t)
<whitequark>
the second thing is definitely a footgun for shapes
<whitequark>
the first one is eh okay
<whitequark>
I thought you referred to the second one at first
<whitequark>
but then I actually read the PR
<awygle>
ah ok
<awygle>
easy fix
<awygle>
wtf are the semantics of < on a tuple anyway...
<awygle>
(1,2)<(2,1)==(true,false)?
jeanthom has joined #nmigen
<awygle>
ah I see, they just go in order
<whitequark>
yep
<whitequark>
convenient
<awygle>
seems fraught to me but oh well
<whitequark>
it... well, is, in the same sense that python is
<whitequark>
the usual weakly typed nonsense
<whitequark>
btw
<whitequark>
`(1,2)<(2,1)==(true,false)` does not do what you think it does
<whitequark>
it does something called "chained comparisons"
<whitequark>
>>> (1,2)<(3,4)==(3,4)
<whitequark>
True
<whitequark>
you might already know that
<awygle>
I would have figured that yeah, I was just trying to convey "what, do they return a tuple of answers?"
<awygle>
.... wait I just processed that. I did _not_ know that.
<whitequark>
:D
<awygle>
python makes me so goddamn angry lol
<awygle>
I'll Google that tomorrow, it's 3am, I shoulda been asleep hours ago
<awygle>
I lied, I Googled it. That's much more sensible than I first thought.
<whitequark>
heh
<whitequark>
sadly we can't do it for nmigen values
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
jjeanthom has joined #nmigen
jeanthom has quit [Read error: Connection reset by peer]
noknok has joined #nmigen
noknok has quit [Client Quit]
jjeanthom has quit [Ping timeout: 240 seconds]
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
<d1b2>
<edbordin> I think I'm gonna ship yosys-smtbmc on windows running on an interpreter included with the distribution (nominally called python3-private.exe to try and avoid anything else trying to use that interpreter). I already had to include the whole lib/python3.x folder for nextpnr anyway
<d1b2>
<edbordin> To the user it should just appear to work as an exe and there should be no unexpected effects from activating different python envs
<d1b2>
<edbordin> also the same approach would work for the sby launcher if the code was made to run on windows again
<whitequark>
seems like a good idea to me
PyroPeter has quit [Ping timeout: 272 seconds]
PyroPeter has joined #nmigen
<kbeckmann>
is it possible to get a warning/build error when just writing "signal.eq(other_signal)" instead of "m.d.comb += signal.eq(foo)" or similar? i've accidentally written this a few times recently for some reason and it led to me having to comb through my changes to find the mistake..
<kbeckmann>
i understand that the expression itself just returns an Assign object, but it would be nice to somehow detect and warn if it isn't consumed by anything.
<whitequark>
kbeckmann: it is possible, but certain deficiencies make it impractical right now
<whitequark>
i actually wrote a patch to do it and then shelved it
<kbeckmann>
i see
<whitequark>
can you open an issue? we can certainly fix it in the future
<kbeckmann>
okay.
<_whitenotifier-b>
[nmigen] kbeckmann opened issue #423: Emit a warning when an Assign expression is not added to a domain - https://git.io/JJLm7
<_whitenotifier-b>
[nmigen] whitequark commented on issue #423: Emit a warning when an Assign expression is not added to a domain - https://git.io/JJLYq
<kbeckmann>
whitequark: a while ago we talked about getting support in nMigen for IOs with xdr=4 and 7 for the ECP5, and you mentioned that you also wanted the phase information included here. i'm not quite sure how to include that. meanwhile i put my changes on a local branch. should i open an issue or PR to discuss this further? my WIP changes are here for the time being
<whitequark>
kbeckmann: there is no particular order in which the two changes (xdr 4/7 and phase info) need to be merged
<kbeckmann>
ah i see. then i will clean it up, update docs and make a PR with what i have for now.
<whitequark>
your changes basically look good at first glance
<kbeckmann>
cool
<whitequark>
naming nit: i'd call the new clock something like `fclk`, for `fast clock`
<whitequark>
i know `e` stands for "edge" which is ecp5 terminologu
<whitequark>
*y
<whitequark>
everything else looks just fine
<kbeckmann>
ok! will update
<whitequark>
ideally the PR would have two commits, one for xdr=4 and one for xdr=7 but you probably know that
<kbeckmann>
yeah, will do that. thanks for letting me know, saves time.
<miek>
i've been wondering whether it should also insert a DELAYG instance for inputs? that's what they recommend in the appnote "Static data delay element DELAYG is used to delay the incoming data enough to remove the clock injection time."
chipmuenk1 has joined #nmigen
chipmuenk has quit [Ping timeout: 244 seconds]
chipmuenk1 is now known as chipmuenk
<kbeckmann>
does it state how much the delay should be? also, there is the ALIGNWD signal that I haven't used at all. maybe it should be exposed to the user as well somehow.
<awygle>
Delayg is fixed delay
<kbeckmann>
ah i see
<daveshah>
You give it a string value
<daveshah>
that auto-selects the right delay for the application
<daveshah>
that is the type to value mapping if anyone is curious
<_whitenotifier-b>
[nmigen] kbeckmann opened pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLCJ
<_whitenotifier-b>
[nmigen] codecov[bot] commented on pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLWk
<_whitenotifier-b>
[nmigen] codecov[bot] edited a comment on pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLWk
<_whitenotifier-b>
[nmigen] codecov[bot] edited a comment on pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLWk
<_whitenotifier-b>
[nmigen] codecov[bot] edited a comment on pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLWk
<_whitenotifier-b>
[nmigen] whitequark closed pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLCJ
<_whitenotifier-b>
[nmigen/nmigen] whitequark pushed 2 commits to master [+0/-0/±3] https://git.io/JJLWG
<_whitenotifier-b>
[nmigen/nmigen] kbeckmann 981e674 - vendor.lattice_ecp5: Add support for io with xdr=4
<_whitenotifier-b>
[nmigen/nmigen] kbeckmann d4946b0 - vendor.lattice_ecp5: Add support for io with xdr=7
<_whitenotifier-b>
[nmigen] whitequark commented on pull request #424: vendor.lattice_ecp5: Add support for io with xdr=4 and 7 - https://git.io/JJLWZ
<_whitenotifier-b>
[nmigen/nmigen] github-actions[bot] pushed 1 commit to gh-pages [+0/-0/±13] https://git.io/JJLWn
<_whitenotifier-b>
[nmigen/nmigen] whitequark 8092217 - Deploying to gh-pages from @ d4946b060a342cb215acca62a6c8ed379efefa75 🚀
lkcl__ has joined #nmigen
lkcl_ has quit [Ping timeout: 240 seconds]
hitomi2504 has quit [Quit: Nettalk6 - www.ntalk.de]
<ktemkin>
so, is this the scheduled nMigen meeting time (or five past), or am I off?
<jfng>
yes, i think
<whitequark>
it is!
<whitequark>
sorry, i'm late
<whitequark>
anyway, let's begin
<cr1901_mobile>
I'm in read only mode I think, but I'm here
<whitequark>
the purpose of these meetings is to sync up with everyone who works closely with nMigen; either on nMigen itself, or on projects that use or would like to use the latest features
<whitequark>
first I'm going to share my roadmap, which many of you probably already know
<whitequark>
the priority is to get a 0.3 release out with cxxsim in it, which should dramatically improve simulation speed for complex designs
<whitequark>
are there any blockers you'd like to see solved in 0.3? PRs I haven't reviewed? things like that
<whitequark>
or any other concerns really
<ktemkin>
I don't have anything I can think of
<cr1901_mobile>
Anything I want can wait until 0.4
<jfng>
wrt. cxxrtl, is there a plan for some sort of generic driver with an event loop ? iirc we talked avec libuv/uvw privately some time ago
<whitequark>
good question, right now the entire thing is driven from Python
<whitequark>
now that I think about it more: since the simulation is passive (as in, it must be clocked externally), what would an event loop that calls back into blackbox code achieve?
<whitequark>
is it just that polling is more expensive due to all the syscalls? portability?
<jfng>
(i haven't tried the blackbox integration with nmigen yet)
<jfng>
is the use case of "I need to drive sync, and have my blackboxes clocked" already supported ?
<whitequark>
there's no usable code that would bridge nmigen simulation API to cxxrtl (yet), but in principle, yes, this should work just fine
<whitequark>
you could `add_clock` and have the black boxes clocked, assuming they are appropriately hooked up
<jfng>
that's nice
<jfng>
on nmigen-soc, PR #19 is a blocker for the peripheral API
<whitequark>
let's take a look at it
<cr1901_mobile>
Will there be time at the end to discuss what we want for new features, or can we chime in whenever?
<ktemkin>
might be good to finish focusing on current/planned work and then move on to future/desired work
<whitequark>
yup
<cr1901_mobile>
Yea wasnt sure whether others were finished (no timestamps)
<whitequark>
jfng: okay, I see two issues with that PR
<whitequark>
how would the event map in the peripheral info be actually used?
<whitequark>
for the memory map, it's clear: you can iterate the resources in the peripheral and, while holding the memory map for the CPU (or another bus initiator), generate the maps from the point of view of that initiator
<jfng>
for the event map, there are two scenarios
<jfng>
the interrupt controller (or the cpu interface, maybe) would gather a top-level event map in order to get IRQ numbers
<jfng>
the BSP generator needs to walk through the event map of each peripheral to get the event numbers, and trigger modes
<jfng>
I think the interrupt controller should actually hold this top-level event-map
<whitequark>
event maps are currently not hierarchical, right?
<jfng>
they are
<whitequark>
I think I'm missing something, but I don't understand how
<whitequark>
EventMap.add only takes Sources
<whitequark>
the *monitors* are hierarchical, yes
<jfng>
yes, and Sources take an optional event_map attribute
<whitequark>
oh I see
<whitequark>
yup, sure, that works
<jfng>
maybe we should improve the nmigen_soc.event documentation, I also get confused sometimes when I re-read it
<whitequark>
so then the problem becomes slightly different
<whitequark>
you have PeripheralInfo, now you want to know (a) the interrupt number and (b) the bit you need to write to to disable the interrupt
<whitequark>
for (b), the event index in the event map local to that peripheral gives you that
<whitequark>
for (a) I'm not sure if there's a way to do this at the moment?
<jfng>
are you talking about the interrupt number assigned to this peripheral ?
<whitequark>
yeah
<whitequark>
where would you get it?
<jfng>
then for (a), you would need to get it from the interrupt controller event map
<jfng>
so one level higher in the hierarchy
<whitequark>
yes, but you don't have the event source in the peripheral info
<whitequark>
for the entire peripheral that is
<jfng>
no, but the BSP generator would have access to the event map of the interrupt controller
<whitequark>
yes, but how would it find out the interrupt number from PeripheralInfo and the event map of the INTC?
<jfng>
ah, I see what you mean
<jfng>
right, this link is missing
<whitequark>
okay, so that was the first issue
<whitequark>
let's go to the second one
<whitequark>
I find the vagueness of ConfigDict somewhat concerning
<whitequark>
specifically I don't actually understand how a BSP generator would use it
<jfng>
the basic idea is to provide a container for constants, as in "key (a string) => constant (anything printable?)"
<ktemkin>
the idea is roughly that e.g. a header generator would be able to iterate over it and dump out peripheral-related constants in header files, right?
<jfng>
yes, that's the intended use case
<whitequark>
the general idea is of course good and necessary
<whitequark>
to be clear I'm only talking about the specifics of this API
<whitequark>
I think "anything printable" won't fly because of the way different languages represent constants, which is in wildly different ways
<ktemkin>
^ agreed with that last statement
<whitequark>
in C you have to append some combination of u and l suffixes to large integers
<whitequark>
in Rust you usually don't
<whitequark>
floats (or more likely rationals) need special treatment in languages without first-class support
<whitequark>
strings also
<whitequark>
so I think we need to do three things here
<ktemkin>
whitequark: so, are you okay with having that kind of "misc const" structure on a peripheral, but not okay with the lack of e.g. typing on the type?
<whitequark>
pretty much, though I am open to arguments against the first part (whether we should have them at all)
<whitequark>
I find the idea that the core must have *some* way to communicate its build-time configuration to the BSP generator convincing
<whitequark>
well, BSP generator and then actual driver code
<ktemkin>
mm
<jfng>
ah, so maybe we should add metadata to values to hint how they should be represented
<ktemkin>
I can see the utility of it; but I'm concerned it might become a junk drawer if the use isn't at least carefully documented
<whitequark>
it is inevitably going to become somewhat of a junk drawer
<whitequark>
totally open to brainstorming ways to improve on that
<whitequark>
let's consider some specific things that I imagine would go there
<ktemkin>
specifically, I'm concerned that we're going to wind up with unintended uses of this becoming part of the required API to Not Break Things
<whitequark>
- the clock frequency (if there's a single specific one) at which a core runs
<whitequark>
- various divisors used inside the core
<whitequark>
- FIFO sizes
<whitequark>
- functional unit counts, for cores with configurable sizes
<whitequark>
I share your concern
<whitequark>
I'm just not aware of a way to improve on this
<ktemkin>
one simple way is to better document intention while adding it
<whitequark>
agreed; that would be my first suggestion
<whitequark>
we shouldn't call it "ConfigDict"; it is not for communicating completely arbitrary things downstream of the design. we should call it "ConstantDict" and it should only carry the constants required for the driver to bring up and operate the peripheral
<ktemkin>
here's another question: is this only specifically for constants global to the peripehral?
<whitequark>
hmm
<whitequark>
what are the other possibilities here?
<ktemkin>
if I have a register that has modes foo (0), bar(1), and baz(2), ideally the relevant constants could be attached to the register in a similar way
<whitequark>
oh right, so that's basically a enum CSR, right?
<ktemkin>
mm
<whitequark>
our current CSR infrastructure is very barebones but I'd like to improve it, and one of the improvements would be to have more or less the same expressive power as SVD for CSRs
<ktemkin>
that seems to me like a very similar problem to tying these little bits of metadata to the peripheral
<whitequark>
can you elaborate?
<ktemkin>
in the end, you have little bits of metadata that can be attached to registers; e.g. you have a counter register with a synthesis-time limit and want to communicate that range via your BSP generator
<whitequark>
ah I see
<whitequark>
so that's a different thing to enums, but it's a reasonable ask!
<whitequark>
I think it would be fine to be able to associate constants both with individual registers, and with an entire peripheral
<whitequark>
that would map well to Rust too
<ktemkin>
mm; I see the requirements here (having a collection of little tidbits of type-annotated data) as being close to the same between the peripherals and their registers; and that leads me to believe it might be a good idea to share code between both cases
<whitequark>
I agree. my second suggestion would be to get rid of the nesting *in* the ConfigDict (or whatever we call it), though for a different reason. given your suggestion, I propose that we split this part of the PR out as something like a ConstantMap and define a few places where it can be attached
<whitequark>
so the constants would still be nested inside other (conceptual) objects, but the hierarchy would be outside of the constant dictionary itself
<ktemkin>
+1
<ktemkin>
that'll also curtail the amount of "junk drawer" clutter that can be accumulated on the peripheral
<whitequark>
my third suggestion would be to strictly limit the types that can be put into it, and define a clear interpretation for them
<jfng>
yeah, this separation makes sense since nesting will only be useful for a handful of select use cases
<whitequark>
we can start with `bool` and `int`, perhaps
<jfng>
i added nesting in order to support complex peripherals like a dram controller that may have a lot of config options
<whitequark>
we can add it later; there are some issues with implementation of nesting, too
<whitequark>
for example, in Rust, you probably want to generate nested modules, which isn't very convenient to do via `all_items`
<jfng>
ah, yes. all_items() is limited to separators
<whitequark>
I'd prefer to add nesting once we actually have some BSP generators up and running and can hack on them and see how it'd look
<jfng>
right
<whitequark>
anyway, to summarize: rename ConfigDict to ConstantMap; move it out into its own module; restrict the accepted types conservatively; remove nesting
<jfng>
what do you means by "clear representation" for the types ?
<whitequark>
ah yes
<jfng>
shouldn't this be the responsibility of the BSP generator ?
<ktemkin>
the BSP generator needs to know type information to do that
<whitequark>
so the problem is that Python has ints, but the BSP generator can't just use ints, or at least, it can't always do that
<whitequark>
in C you can sort of #define an integer constant (it won't always work but it will most of the time)
<whitequark>
in Rust there's no #define (ok I guess you could use macro_rules but that's horrible)
<whitequark>
you have to know whether you're making an u32 constant or an i32 or an usize or whatever
<whitequark>
so the ConstantMap actually needs to be strongly typed
<ktemkin>
mm
<whitequark>
it needs to have a ConstantType enum and it needs to check the value against the range provided by its type
<whitequark>
we can't just have the BSP generator guess the type based on the magnitude for example, since while that'll mostly work in C, it will break Rust code badly
<ktemkin>
and sometimes, those constants may be things that need to be fed back into e.g. a CSR, in which case we'll have to specify a way to provide an exact binary representation, like bits+width
<whitequark>
that's a bit tricky because none of the languages I imagine we'd have BSP generators for actually support arbitrary width integers
<whitequark>
I think the best we can do is to have the BSP generator determine the type returned/accepted by the accessors, and the type of the constant, in the same way
<whitequark>
(I hope most of these cases can be handled without a fuss through CSR enums)
<jfng>
i see
<jfng>
i'll try to iterate on the PR shortly. many thanks to both of you for the live review :p
<ktemkin>
whitequark: one case I'm thinking of immediately is having a BSP that generates an SVD itself
<ktemkin>
that seems like a good example BSP generator to consider when imagining something that's going to run into all the PITA cases
<ktemkin>
(and one that'd be desirable due to how many different tools accept SVDs)
<jfng>
i was thinking of generating a device tree file, as a sophisticated use case for the BSP generator
<whitequark>
ktemkin: indeed that is also what I'd like to have
<whitequark>
it's been a while since I read the spec, but I believe SVD does actually have enums
<ktemkin>
yep
<ktemkin>
(it also likes to know exact widths for things)
<awygle>
shit, i showed up at 10, there was no meeting, i assumed i had my dates wrong. sorry.
<whitequark>
ktemkin: ah that's good to know, so maybe we shouldn't have an enum but something more precise for the types
<whitequark>
jfng: I think we can land the PeripheralInfo and ConstantMap PRs separately; the latter seems like it'd take a bit more iteration
<whitequark>
awygle: ah oops, sorry. I actually missed my own alarm for some reason ...
<whitequark>
anyway, I think you can go for it now? it's been fairly long but I have enough time right now, not sure about others
<ktemkin>
https://paste.debian.net/1155401/ <-- here's an example of the information we might provide for a general register field in an SVD
<whitequark>
right, that makes sense
<whitequark>
but I think this doesn't specify the width of the enumeration values *themselves*?
<whitequark>
so because of the way CSR works / should work (not all of it is implemented right now), the BSP generator will of course know the precise bit positions of fields
<awygle>
whitequark: the meeting started at 11 local so it's my fault not yours
<whitequark>
ahh
<awygle>
catching up on backlog...
<whitequark>
and here I was thinking UTC is unambiguous :D
<awygle>
It is, I just suck :-P
<ktemkin>
awygle: wait, are you Eastern time?
<awygle>
PST
<whitequark>
ktemkin: so in Rust that enum would have to be represented by u8
<ktemkin>
ah, okay, it was someone else who was Eastern
<whitequark>
cr1901_modern was in EST maybe?
<awygle>
or PDT? i dunno. whichever.
<ktemkin>
whitequark: yeah -- my thought was for something like that, we'd say N bits and let the BSP generator select u*
<ktemkin>
*u8
<whitequark>
enumerated values are a bit special though because then the type of the field is just the Python Enum/IntEnum
<whitequark>
do we want something like that for regular constants too?
<whitequark>
or would it be sufficient to have a choice of u*/i*?
<ktemkin>
for a generic constant, we might want to have something like `bits(0x01, 3)`, and then the BSP just goes "oh, yeah, that's a u8"
<ktemkin>
dunno; this might be overly bikeshedding at this point, and maybe we should just go ahead and run into issues during implementation
<whitequark>
I sort of see the utility but it also feels extraneous
<whitequark>
anyway, we can decide it later, I think
<ktemkin>
mhm
<cr1901_modern>
whitequark: Yes, EDT
<whitequark>
I don't actually know any target language where it'd help
<whitequark>
it might help debuggers, maybe?
<ktemkin>
I was thinking of two things
<ktemkin>
debuggers were one of them
<ktemkin>
the other I think is now too esotetic to bother with, so mostly yeah: debuggers
<whitequark>
I can see another use case
<whitequark>
you might have some predefined values for a CSR field that do not actually map to a enum
<whitequark>
maybe a calibration value or something
<ktemkin>
mm
<whitequark>
or a bitfield (we should probably have first-class support for that)
<whitequark>
(ok no that's dumb, we can just add more CSR fields)
<whitequark>
or a mask of some sort
<whitequark>
it's kind of marginal
<whitequark>
I think we can always start with a fixed list of U* and then expand it to a more flexible representation and make the old names aliases
<ktemkin>
mm
<whitequark>
so I think there's no urgent need to decide on it
<agg>
svd fields can have either enums or "write constraints" for a field, the latter can say a min/max value for example
<agg>
usually those two options cover most csr field needs
<whitequark>
oh, good to know
<ktemkin>
anyways, did we want to move on?
<whitequark>
yep
<whitequark>
awygle and cr1901_modern had something to say I think
<cr1901_modern>
It can wait until I have something tangible.
<whitequark>
ack
<cr1901_modern>
I wanted to query whether you're still interested in paramiko support
<whitequark>
oh, sure
<whitequark>
it should be pretty straightforward, I think you can just send a PR
<awygle>
i wanted to hear your plans for sby, as well as what would constitute a 0.4
<whitequark>
0.4: just cxxrtl + whatever minor misc stuff we can get in in that timeframe
<whitequark>
people *really* need cxxrtl for larger designs
<awygle>
i thought cxxrtl was 0.3?
<whitequark>
er
<whitequark>
sorry, I misread
<whitequark>
0.4: split records into packed structs and interfaces, design streams
<whitequark>
well, plus write a shitton of docs, but that's entirely on me
Asuu has joined #nmigen
FFY00 has quit [Remote host closed the connection]
<awygle>
ok. i'm very interested in "design streams" but that can wait for another day :)
<whitequark>
yeah, next meeting perhaps?
<awygle>
where would the FSM stuff fall?
<whitequark>
we're going to have to tackle Records first as that's really critical for streams
<awygle>
mhm
<whitequark>
(in my mind a Stream would be a PackedStruct payload in an Interface)
FFY00 has joined #nmigen
<awygle>
not any particular FSM stuff mind you, just "Address the need for a better FSM DSL" in general. 0.5?
<whitequark>
0.5 seems fine
<awygle>
mk
<whitequark>
I'd also like to refactor the innards of elaboration somewhere along 0.5-0.6 range
<awygle>
yeah you've talked about that several times before
<whitequark>
and after that it's probably 1.0 time
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
<whitequark>
unlike Rust, which will be 1.x forever, I was thinking we'd increment major version on a regular schedule, and drop anything we deprecated on the same schedule
Asu has quit [Ping timeout: 272 seconds]
<whitequark>
once a year at first, maybe less often than that later. not a concrete plan but more of a feeling of where I'd like to be
<awygle>
<3 <3 <3 yessssss thank you
<whitequark>
the idea is that the amount of deprecations/removals would be small and manageable, but at the same time we wouldn't be locked into our early choices forever
<whitequark>
EDA landscape is changing fairly quickly as of late
<awygle>
plus several thousand from me. probably surprising no one.
<ktemkin>
here's a minor "would be nice" feature before I have to run: some conveniences for running code examples on multiple platforms with varying I/O resources
<whitequark>
oh, sure; what would you like to have?
<whitequark>
right now the blinky example just catches the error
<ktemkin>
In LUNA, I have a little bit of syntactic sugar in `request_optional`
<whitequark>
ah so just fold the try..catch into the platform?
<whitequark>
oh I see, so it'd return a dummy device
<whitequark>
that's unfortunately troublesome, because it does not generalize to subsignals
<awygle>
this seems related to the "simulate stuff that needs a platform" case
<ktemkin>
right now, in that quick convenience on my side, it just spits out an equivalent Signal() if the I/O's not there
<ktemkin>
but ideally it'd be something that generates stand-ins for subsignals, yes
<whitequark>
(there's another related issue in that you're .eq()ing a whole Pin, which is a footgun I should handle)
<whitequark>
well
<whitequark>
the problem is that you have to actually describe the subsignals somewhere
<ktemkin>
yep
<whitequark>
I think that ironically of all the things we discussed, this is the hardest to implement
<whitequark>
or rather to design
<ktemkin>
I was actually thinking it might be something that just creates a "black-hole" proxy
<whitequark>
this seems like it'd be hard to debug if you e.g. make a typo
<whitequark>
with a try..except you can just add a print() or something
<whitequark>
there might be an entirely different approach to achieve the same convenience
<whitequark>
I'm not sure which, yet, but I'll keep it in mind
<ktemkin>
mm; I'm not recommending this one
<ktemkin>
I just created it quickly as a scaffold for a Correct solution
<whitequark>
I do see the verbosity of wrapping all those in try..except
<whitequark>
it's unfortunate
<ktemkin>
mm
<whitequark>
a potential solution would be to make Pin an Interface and then add something like request_connect that can be optional
<whitequark>
then the inner structure is described in your code (that provides the interface to be connected to the pin)
<whitequark>
(we probably need the first part anyway, Pin should not be a packed struct that can be assigned as a whole)
<ktemkin>
another simple solution might be just to have something like a default= argument for something like `request_optional` -- it's what you get instead of the pin's not there
<ktemkin>
that'd give an immediate solution (e.g. I can grab a correctly-shaped record for now), and then later there can be proper interface-specs that are squished in there
<whitequark>
mm, I'll think about it
<whitequark>
does LUNA need this specifically in the core?
<ktemkin>
no, but having it in core nMigen would also solve the "I need to replicate I/O during simulation" case
<ktemkin>
at least, I imagine
<whitequark>
hmm
<ktemkin>
and I'd like to stop by terrible habit of carrying around my own platforms >.>
<ktemkin>
*stop my
<whitequark>
so .request_optional doesn't need to be a part of a platform, it can be a free function
<whitequark>
(LUNA has its own board, right? is that one upstream yet?)
<ktemkin>
in nmigen_boards? nope
<whitequark>
regarding simulation with a platform, we already have an issue for that! we can try to address it for 0.4 I think
<whitequark>
is there any reason you haven't submitted it?
<ktemkin>
because until very recently, only like ~5 people have had boards
<ktemkin>
so "hardware's not quite finalized"
<whitequark>
ahh gotcha
<whitequark>
that makes sense
<whitequark>
I wasn't sure if it was something like that ornot
<cr1901_modern>
Hopefully, adding platform simulation _in the long run_ will almost be as seamless as adding a platform file period :D
<awygle>
Is the intent for bespoke boards to go into nmigen_boards? Or only freely available dev boards?
<whitequark>
anything generally publicly available at some time
<whitequark>
in general I don't think we are going to run out of space in nmigen_boards somehow
<whitequark>
changing resource definitions inflicts a cost, but... this cost is finite and relatively small
<whitequark>
there are not that many interfaces in the world and we can only screw them up so many times
<awygle>
Lol
<awygle>
I'll keep that in mind if I ever finish this project then
<whitequark>
consider opam
<whitequark>
the *entire ocaml package repository* is one github repo with one team of maintainers
<whitequark>
and they actually run CI globally on the entire set of packages
<whitequark>
sure, this approach doesn't scale *infinitely* but it scales much better than one might think
<ktemkin>
if we run into a problem with too many boards supported, that's a good thing, and we can figure things out then :)
<vup>
whitequark: kbeckmann: I saw that xdr={4,7} support was added for ECP5. Am I reading the datasheet correct in that the fast clock (edge clock) would be a DDR clock for these? (So for xdr= 4 fclk would have twice the frequency of the clk)
<vup>
Also currently there seems to be no way to specify if one wants xdr with a DDR fast clock or with a SDR one, which is something 7series supports. Do you think this is something worth adding?
<daveshah>
Yes, the fast clock is DDR only for ECP5
<vup>
daveshah: I see, thank you for confirming that. Might be nice to add to the Pin doc string.
<whitequark>
the Pin abstraction gets stretched pretty thin here
<whitequark>
we're going to have to migrate off it at some point
<vup>
yeah, currently I think the way the ecp5 stuff is done kind of blocks adding xilinx support, as it only does 7:1 with a SDR clock
<whitequark>
hrm
<whitequark>
SDR here meaning that the period equals 7 bit times?
<vup>
no, the 1 bit time
<vup>
s/the//
<whitequark>
right so we could add two attributes to Pin
<whitequark>
the phase offset between clk and i/o, and the multiplier needed to drive fclk
<vup>
right, that would work
<whitequark>
this is of somewhat limited usefulness until we have a PLL abstraciton
<vup>
sure, but it allows people to start playing around with PLL abstractions which might help the design process on that
<whitequark>
indeed
<whitequark>
I still think we should do it, the only way to figure out if it's viable is to try
<vup>
yeah
<vup>
also it might be worth to consider adding two differently named clock inputs, or something that forces the user to check the multiplier / phase, as otherwise this feels like a bit of a (beginner?) trap
<whitequark>
we already do have them, right?
<whitequark>
oh, for sdr/ddr you mean?
<vup>
yes
<whitequark>
we have the same problem with phase
<whitequark>
well
<vup>
true
<whitequark>
beginners aren't going to get XDR>2 to work anyway
<vup>
a UnusedClockMultiplier / UnusedPhase warning might be a better option
<whitequark>
so it's safe to assume anyone seriously attempting to get that to work would have read the docs
<vup>
whitequark: well I was more thinking about somebody moving from a 7series device to a ecp5. From the 7series one might assume, that 7:1 serdes has a sdr clock and with the instance name that contains ddr hidden it feels like easier to miss the change
<whitequark>
ah right
<whitequark>
it's not trivial to add that warning; let's see how people do portable designs first, and then adjust to that
<daveshah>
For ECP5 there are also some extra device specific DRC issues
<daveshah>
Like needing to use CLKDIVF to produce the low speed clock
<whitequark>
right
<daveshah>
And in many cases ECLKSYNC and a specific startup FSM to guarantee reliable behaviour
<whitequark>
it's unlikely we can completely eliminate vendor-specific code
<vup>
whitequark: sounds reasonable, its just something that came to mind when I saw the pr for the ecp5 support
<whitequark>
so in long term I'd want to provide an IO toolkit more than a complete fixed IO solution
<daveshah>
Indeed, I think Xilinx has clock topology requirements too
<daveshah>
At least in some cases
<whitequark>
current .request() is looking like the latter but that wasn't ever the intent
<whitequark>
the implementation is just flawed
Asuu has quit [Remote host closed the connection]
<_whitenotifier-b>
[nmigen] alanvgreen synchronize pull request #420: Update license and copyright info - https://git.io/JJTYQ
<_whitenotifier-b>
[nmigen] codecov[bot] edited a comment on pull request #420: Update license and copyright info - https://git.io/JJTYN