<GenTooMan>
reminds me of a girl in high school she got kicked by her horse... she got a few ribs cracked. lots of complaining on her part.
peepsalot has quit [Ping timeout: 246 seconds]
peepsalot has joined #nmigen
programmerjake has joined #nmigen
<programmerjake>
double checking: m.If/m.Switch/m.Case are all passed through unmodified by nmigen to rtlil processes which are converted to mux trees by yosys, right?
<RobTaylor[m]>
I’m sure lkcl told me last night that there isn’t a case where it doesn’t go down to mux, just that there’s an interveneing case,
<programmerjake>
* whitequark: are vup and I correct? lkcl thinks vup is wrong...
<RobTaylor[m]>
So colour me confused ;)
<programmerjake>
I'll wait till whitequark replies...otherwise I may end up inadvertantly giving a false explanation...
<miek>
it's easy to check, build a design and look at top.il
<programmerjake>
yeah, true, i'll still wait for an authoritative source since hypothetically nmigen could do the translation only sometimes...
<mwk>
what exactly is the question here?
<dragonmux>
lkcl is wrong at least on some synth tools in that how a switch in the RTLIL/Verilog is mapped is up to the synthesis tool - if you have an m.If gating a sync domain assignment, while this is mapped to a switch in the input to the synt step, it's then mapped to a DFF enable line for iCE40 and Xilinx flows
<programmerjake>
in the process of nmigen producing rtlil and yosys processing it, at some point If/Switch/Case are translated from control-flow into muxes... lkcl thinks that happens in nmigen so we could add code to nmigen that hooks into the post-mux-conversion nmigen IR and does some custom modifications before emitting rtlil, whereas I and vup think that that is left to yosys's process passes to generate muxes so hooking nmigen's IR won't work
<dragonmux>
this is a very neuanced topic that has no single "one right rule"
<mwk>
programmerjake: yeah, nmigen relies on yosys process logic, it definitely doesn't lower If/Switch to muxes itself
<mwk>
(which is a source of annoyance, by the way)
<mwk>
in fact, there are paths that can be taken in yosys that will cause a $pmux or $shiftx to be used instead of $mux
<programmerjake>
yeah, i'm going to wait for whitequark's answer here, since lkcl didn't accept my appeal to vup's authority...
<vup>
(not that i have any authority :P)
<dragonmux>
(to be clear, our m.If example is and if-without-else-or-elif, where it becomes clear to the tools that you're asking for an enable)
<dragonmux>
in this case, we've read the code and checked the output.. mwk authors some of the flow tools.. our answers can be considered authoratative
<mwk>
dragonmux: in that case, there is still a $mux in there, it just happens to be folded by later optimization passes
<dragonmux>
not on the Xilinx flow
<mwk>
hm
<mwk>
you mean with vivado synth?
<dragonmux>
their synt takes the switch in and immediately synths' a DFF enable
<dragonmux>
mhm
<mwk>
heh
<dragonmux>
just been doing far too much with that recently and having to read the results, so
<mwk>
right, that sidesteps a good chunk (but not all) of yosys' proc pass-set
<dragonmux>
aye
<dragonmux>
and uses yosys write_verilog to generate the input file for vivado which then applies its own rules and synthesis
<mwk>
yeah, then there really is no $mux (for an If/Switch) in this flow at any point
<dragonmux>
exactly
<dragonmux>
same thing also applies for the Spartan 6 ISE flow
<dragonmux>
oh, talking of.. we (lethalbit and us) have a patch for your PR for the Xilinx flows mwk - it doesn't take the new Kria SOM modules into account
<dragonmux>
the patch changes line 461 from `elif device.startswith("zu"):`
<dragonmux>
to `elif device.startswith(("zu", "k26")):`
<mwk>
wait what
<dragonmux>
that's tested to work
<dragonmux>
think it was you who wrote the currently open PR that combines the Xilinx flows
<mwk>
ohhh
<mwk>
#563, right
<dragonmux>
mhm
<dragonmux>
we've been using it because we wanted to test it
<dragonmux>
that's the only issue we've found
<mwk>
what's the exact device string you pass to vivado for that part?
<dragonmux>
xck26
<dragonmux>
the entire part number is
<dragonmux>
`xck26-sfvc784-2lc`
<mwk>
... how annoying
<dragonmux>
*lv, not lc
<dragonmux>
yeah, hence how we handled it with that particular elif modification
<mwk>
any other kria stuff to be handled?
<dragonmux>
took a good while staring at the code to figure out how to fold it in
<mwk>
I see Alveo isn't handled either right now
<dragonmux>
all of the K26 SOMs use that same `xck26` base, so they should be ok
<mwk>
xcu<number>
<dragonmux>
mmm
<dragonmux>
we've not poked those, so
<mwk>
me neither, but I'm very sure these are just rebadged other ultrascaleplus devices
<mwk>
so, trivial change
<mwk>
anyway
<dragonmux>
aye, figued it was worth letting you know though
<mwk>
I'll rebase that PR, I guess
<mwk>
any other stuff that needs changing in there?
<dragonmux>
not that we found that's not a complex (seperate PR) defficiency in nMigen to do with supporting the PS blocks
<mwk>
hm, what about those?
<dragonmux>
the buffers produced for various things require generating BIBUF rather than IBUF/OBUF/IOBUF (and these are not translated for us) and this makes for a very complex pin map and some mess being required as all MIO (fixed-IO) and core pins (including DDR) need this applying, but where those are depends on the part and package
<dragonmux>
we're developing a patch to the unified flow to make that happen but it'll take time to get right
<dragonmux>
the trickiest thing about BIBUF is.. they don't really exist, they just exist to get the right constraints checked so the tool can verify everything and generate the FSBL init code
<mwk>
BIBUF?
<mwk>
... I didn't need those in my code
<dragonmux>
it's a PS-special bidirectional buffer
<mwk>
but then I didn't bother with vivado's FSBL generator
<dragonmux>
you don't need them for EMIO
<mwk>
so maybe that's why I didn't notice
<dragonmux>
but MIO, DDR and core clocking you do
<mwk>
not for generating the bitstream at least
<dragonmux>
we plan on using them to generate the FSBL from nMigen as a Zynq specific output
<mwk>
okay, that's more involved, I guess
<mwk>
you want to use Vivado for that?
<dragonmux>
we want to cut vivado out from that because the code it generates is terrible
<mwk>
hm
<mwk>
will continue this discussion later, SIGGF
<dragonmux>
mhm
<gatecat>
6:24 PM <mwk> me neither, but I'm very sure these are just rebadged other ultrascaleplus devices
<gatecat>
can confirm, built blinky bitstreams for the vu13p and xcu250 a while ago and the only difference between the two as the IDCODE
peeps[zen] has joined #nmigen
<d1b2>
<dub_dub_11> > any other stuff that needs changing in there? @mwk : I may have been too late and I'm not sure if we're talking about the same PR, but I had a small patch to the xilinx-platform branch from a while back
peepsalot has quit [Ping timeout: 264 seconds]
<d1b2>
<dub_dub_11> ah yeah it was also the same PR
<d1b2>
<dub_dub_11> it was something to do with certain characters not being allowed by ISE in certain places iir
<d1b2>
<dub_dub_11> *iirc
<d1b2>
<dub_dub_11> yeah literally just changing a single / to a __
supersat has joined #nmigen
<supersat>
hi all... I'm experimenting with LUNA and trying to drive a second PLL from the main clock input pin. The obvious thing results in "nmigen.build.res.ResourceError: Resource clk_60MHz#0 has already been requested" ... surely there's a way to share an input signal/clock? I'm just not sure how to do it in nmigen
<tiltmesenpai[m]>
I think that depends on the FPGA in question, some of them have special rules for routing clocks
<tiltmesenpai[m]>
pretty sure you might have to go messing around with the PLL settings, there might be an output that's a direct passthrough of the input clock?
<vup>
I have no overview about LUNA, but is the main clock input pin assigned to a `ClockDomain` somewhere?
<vup>
then you can simply get the clock signal using `ClockSignal("domain_name")`
<tiltmesenpai[m]>
on the LUNA hardware? I think the clock feeds directly into a PLL, and the outputs of that are used as the clock domain
<vup>
oh
<vup>
if thats the case, then you probably cannot currently get the input clock
<tiltmesenpai[m]>
can the ecp5 drive a PLL from a PLL?
<vup>
yes
<lkcl>
programmerjake: or, more to the point, you didn't provide vup with sufficient context
<vup>
possible clock input source for the PLL include the "FPGA Fabric" and "Primary Clock Routing"
<vup>
*sources
<tiltmesenpai[m]>
ah, I'd drive a second PLL from the 60mhz domain then
<lkcl>
the idea is to create *separate* (columned) IR-level switch statements, one per "partition"
<lkcl>
this entirely behind a redirected Switch statement
<lkcl>
at line 447 of dsl.py
<lkcl>
Switch(....)
<lkcl>
to be replaced with
<lkcl>
if_test->__Switch__(...)
<lkcl>
the default (normal) Value.__Switch__(...) would do exactly what line 447 presently does
<lkcl>
the PartitionedSignal.__Switch__(...) would perform a subdivision (columnisation) of all inputs and outputs, and create a for-loop around *COMPLETELY SEPARATE AND INDEPENDENT* IR-level Switch/Case statements
<lkcl>
thus yosys IS IN NO WAY REQUIRED TO KNOW ABOUT THIS IN ANY WAY SHAPE OR FORM
<vup>
(what you are describing is pretty much what I meant, when I said my first idea would be to build this into _pop_ctrl)
<supersat>
tiltmesenpai[m]and vup: thanks... I wasn't sure if i could drive one PLL from another so I'll try that
<lkcl>
vup, ah ok, great. sorry, i misunderstood
<lkcl>
there's two areas that need "an abstraction API" (i've analysed the m.If case so far, not the m.Switch/m.Case in _pop_ctrl)
<lkcl>
a careful but mind-bending analysis (and associated actual experiments and unit tests) should show and confirm that a simple independent per-SIMD-partition *duplication* of the IR switch cases (each one of which receives *one* bit from each of the cases), should be perfectly sufficient
<programmerjake>
unfortunately, just duplicating the If statement bodies isn't sufficient, since you'd end up overwriting whole Signals assigned to inside the bodies rather than just the lane desired
<programmerjake>
therefore a whole body transformation is required, which is essentially as complex and does much the same things as the originally proposed vectorization/duplicating-yosys-process-passes
<d1b2>
<dub_dub_11> literally just changed one line
<d1b2>
<dub_dub_11> but what was there before did something funny to ISE where it didn't play well with the / seperator for module hierarchy
<mwk>
hmmm
<mwk>
there are 3 other hierarchy() calls in immediate vicinity
<mwk>
what about those?
<_whitenotifier-d>
[nmigen] mwkmwkmwk synchronize pull request #563: Unify Xilinx platforms into a single class, support more devices - https://git.io/JLcUA
<d1b2>
<dub_dub_11> I think those didn't need changing for some reason
<_whitenotifier-d>
[nmigen] codecov[bot] edited a comment on pull request #563: Unify Xilinx platforms into a single class, support more devices - https://git.io/JLcT8
<_whitenotifier-d>
[nmigen] mwkmwkmwk commented on pull request #563: Unify Xilinx platforms into a single class, support more devices - https://git.io/Jz4iD
<d1b2>
<dub_dub_11> possibly treating some as strings and trying to convert that value into a number and failing because of the /
bvernoux has quit [Quit: Leaving]
<mwk>
I'm not comfortable rolling this change into this PR, particularly since I don't quite understand what's happening
<mwk>
(I've thrown in the xck26 / xcu* thing because it actually affects the code that I *wrote* as part of the PR)
<_whitenotifier-d>
[nmigen] codecov[bot] edited a comment on pull request #563: Unify Xilinx platforms into a single class, support more devices - https://git.io/JLcT8
<d1b2>
<dub_dub_11> I think I did discuss it when I found it, but it would have been a while ago. I'll try find the old messages
<mwk>
either way, I'd suggest doing it in another PR
<_whitenotifier-d>
[nmigen] codecov[bot] edited a comment on pull request #563: Unify Xilinx platforms into a single class, support more devices - https://git.io/JLcT8
<mwk>
though that'd require either landing this PR first, or another rebase...
<d1b2>
<dub_dub_11> ok -direct to the main repo?
<_whitenotifier-d>
[nmigen] codecov[bot] edited a comment on pull request #563: Unify Xilinx platforms into a single class, support more devices - https://git.io/JLcT8
<mwk>
hmm
<mwk>
whitequark: do you think we can get this PR merged sometime soon?