<ZirconiumX>
adumont: Try replacing the = with a <=
<adumont>
I use always the same ram.v as in the "LATTICE, Memory Usage Guide for iCE40 Devices" Technical Note TN1250, June 2016, Appendix A. Standard HDL Code References, p20
<ZirconiumX>
Yosys can't infer single-port RAMs
<ZirconiumX>
Either instantiate it directly or use a dual-port RAM.
<daveshah>
In this case I think it should be fine as it will just end up as a dual port RAM
<daveshah>
Which I think is what is desired
<ZirconiumX>
Given the "** Single-Port RAM **" comment, I disagree, daveshah
<daveshah>
But if this is on an iCE40 other than the UP5K then all the ram is dual port
<daveshah>
And given there is an initialisation, it coudd never map to SPRAM
<daveshah>
The = definitely looks dodgy though regardless of what Lattice say, that should be <=
<adumont>
ok will try to change
<ZirconiumX>
And since the = line is the read port, perhaps that's why Yosys is failing to infer?
<daveshah>
I don't think it should make a difference, but it is definitely not the expected pattern
<adumont>
doesn't make difference, still no BRAM infered. I'm worried about what ZirconiumX says above "it's decided fontRom and labelsRam both have no read ports"
<daveshah>
I will take a look, just rebuilding Yosys
<daveshah>
Looks like the RAMs are being optimised away for some reason
<daveshah>
You say the design works correctly in simulation?
<adumont>
yes it does. the design is simple, it write some labels (strings are in ram (more rom, as I don't use the write ports)., and the font is also in another rom.
<adumont>
that's weird it wouldn't detect the memrd ports, when what is not used in the design are the memwr ports)
<daveshah>
The problem isn't to do with the memrd ports at all
<daveshah>
It looks like something much further away is being optimised away
<adumont>
Do you believe it should appear in the yosys log before MEMORY_COLLECT then?
<daveshah>
Yes, but it is probably not that easy to spot
<daveshah>
as it is hard to know what is even being optimised away firsr
<daveshah>
*first
<adumont>
Is there any way in yosys to disable these optimizations and maybe enable them one by one and see where it start to fail?
<ZirconiumX>
You can run each step individually
<ZirconiumX>
If you manually copy and paste them in interactive mode
<adumont>
yes, but until the end I don't know if it will infer BRAM ?
<adumont>
no sure I follow you :(
<ZirconiumX>
Yosys is a series of passes on an internal representation of your code.
<ZirconiumX>
So you can run one pass at a time and then look around
<ZirconiumX>
For example, if you run `stat`, it will print statistics about your netlist at that point in time
<ZirconiumX>
So if you run a pass and then use `stat`, in the good path you should see a $memrd that becomes $mem and then SB_RAM_4K (IIRC)
<ZirconiumX>
In the bad path, either the $memrd disappears, or else never appears in the first place
<daveshah>
The problem seems to be vgaLabel3.out[23] being stuck low meaning that the if ( ab ) never activates so the $memrd is swept away as the value is never used
<adumont>
@dave
<adumont>
ups
<adumont>
do you see that in the yosys log??
<daveshah>
Not directly, but by piecing together what was going on combined with tracing the ilang (internal Yosys representation) after `hierarchy -top top; proc; flatten; clean;` (i.e. elaboration but minimal optimisations)
<adumont>
you're looking at it or do you do that mentally???
<daveshah>
Looking at it
<daveshah>
it needs a bit of following
<daveshah>
In this case, I can see the driver for \vgaLabel3.out is a register
<daveshah>
the input to that register is { $techmap\vgaLabel3.$0\out[43:0] [43:23] \vgaLabel2.out [22:0] }
<daveshah>
the upper bits are $techmap\vgaLabel3.$0\out[43:0] [43:23]
<daveshah>
oh wait
<daveshah>
it's not directly stuck low
<daveshah>
hmm
<daveshah>
it's tied to \vgaLabel2.out [43:23]
<adumont>
each of the vgaLabel take a 44 bit input and outputs a 44 bit output , it may have modified some bits in the way.
<daveshah>
It looks like in each three modules, none change bit 23 of out
<daveshah>
which means that bits 43:23 of the vgaLabel1 register are always 0 (when S is 0 the mux output is A)
X-Scale` has joined #yosys
<daveshah>
It looks like all three .active signals are ending up at 0
X-Scale has quit [Ping timeout: 258 seconds]
X-Scale` is now known as X-Scale
* adumont
is checking
<daveshah>
Looks like you have a multiple assignment on tmp that is messing things up
<daveshah>
(you assign tmp both in the wire declaration and in an assign)
strobokopp has joined #yosys
emeb_mac has joined #yosys
<daveshah>
Fixing that (removing the assignment in the declaration) and memory is inferred fine
<adumont>
damn!! you're right. I wonder how it's possible verilator nor yosys warn me on that :D
<adumont>
that's an aweful mistake! :'(
<daveshah>
Yosys does warn, both of us just missed the warning
<adumont>
Thank you so much for the help
<adumont>
oh does it?
<daveshah>
I'm surprised Verilator allows this though
<daveshah>
Hmm, I thought I saw a warning
<daveshah>
Yeah, looks like the warning is lost when doing synth_ice40
<daveshah>
I only saw the warning when I was doing some of the early commands manually
<adumont>
are the steps you took described somewhere? I'd love to be able to find out myself the next time (or learn something in that direction, even if I don't get to the same capacity ;) )
<daveshah>
Not really. But I have a pretty standard procedure which is to do -p "read_verilog -lib +/ice40/cells_sim.v; hierarchy -top top; proc; flatten; clean; stat; write_ilang top.il"
<daveshah>
this then gets you a low level dump of what is going on inside Yosys just after initial processing of the input
<daveshah>
then steps can be added and removed as needed
npe has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
<adumont>
thank you so much Dave (and also ZirconiumX) for the help. I'll try myself to get to the conclusion :D with you steps
npe has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
anticw has quit [Remote host closed the connection]
anticw has joined #yosys
<whitequark>
daveshah: i think i filed an issue for this a long time ago and Claire's response was basically WONTFIX
<whitequark>
even had a branch with an attempt to resolve it partially
<mwk>
iirc one issue here is that `check` doesn't detect the case when something is driving a net connected to a constant as a driver-driver conflict?
<whitequark>
that was one of those things that convinced me that improving Yosys Verilog frontend is a dead end and frankly I would just be better off staying away from FOSS Verilog development altogether
<tpb>
Title: `check` does not appear to detect many cases of multiple drivers · Issue #545 · YosysHQ/yosys · GitHub (at github.com)
<daveshah>
I don't think there is a multiple driver by the time check is reached
<whitequark>
actually there is a braindead clause in Verilog which *requires* you to prefer a constant driver
<mwk>
*whaT*
<whitequark>
let me see if i remember it correctly
<az0re>
I don't understand why you think that would be braindead
<whitequark>
az0re: because a driver conflict should be a compilation error
<whitequark>
in the same way, the SV LRM does not allow code that violates the requirements of always_ff to be an error, it has to be a warning, as written
<whitequark>
(so Yosys is not SV compliant here because it actually does make it an error)
<mwk>
... because when you have more than one driver in the first place, you're already doing stuff you shouldn't be?
<daveshah>
My guess is that this was written from a simulation point of view
<mwk>
(with the exception of internal tristate logic but uhhh can we pretend that doesn't exist)
<daveshah>
where there are occasional cases where you do want to override a signal
<az0re>
whitequark: I agree that it is an error, of course
<whitequark>
daveshah: yes, and that should just use a separate keyword that's invalid in synthesis
<az0re>
But I don't understand why it's brain dead to say, "well, this is wrong, but let's be conservative and assign it the constant driver"
<mwk>
because if obvious errors are warnings and not errors, you end up with shit code
<whitequark>
^
<daveshah>
I suspect most of these cases effectively already do, because they use "assign" within a procedural block
<az0re>
mwk: totally agree there, it should be an error with some option to allow it with a warning
<whitequark>
good tools do not second-guess my intent when encountering questionable code. good tools fail loudly on it
<az0re>
sure, agreed
<mwk>
would you also like an option for invalid pointer access in C to be a warning?
<az0re>
I just don't think it's brain dead, *if you're not going to die*, to make that choice
<daveshah>
In fact there is a force keyword already
<az0re>
you misunderstand what I'm saying lol
<whitequark>
oh, you were nitpicking a literal interpretation of my words. nevermind then
<az0re>
huh?
<az0re>
I didn't understand why you thought it was braindead to prefer a constant driver
<az0re>
Turns out you really just have a problem with it not being an error. I agree there.
<whitequark>
right, so
<whitequark>
I hold my tools to a much higher standard than is considered normal in this industry
<az0re>
I don't see how I was nitpicking anything
<whitequark>
yes, I misunerstood your response, sorry
<az0re>
OK no problem I just don't want any lingering misunderstandings
<whitequark>
in general, I think that something like the missing diagnostic we discussed above, in a language specificaion--an unforced error--is completely unacceptable
<whitequark>
in the same sense that the situation we ended with in PCs where everything runs on memory-unsafe code that contains untold quantities of severe bugs is completely unacceptable
<az0re>
TBH I don't see a big problem with diverging from the official Verilog spec (ideally with an option for full spec compliance) to implement more sane behavior in Yosys
<az0re>
I mean, Yosys already picks and chooses things from SV to support, right?
<whitequark>
there is a particular point of view that IME many engineers hold that "Verilog is fine, you just need a linter and a year to learn every footgun it has", and I refuse to concede with it
<az0re>
totally agreed that is a garbage perspective :)
<whitequark>
yes, I agree that Yosys should diverge from the SV spec when it makes sense to do so
<az0re>
and also that Verilog is not ideal...
<whitequark>
I also find it really weird that there is no synthesis subset of SV
<whitequark>
sure, 1164.1 was to put it mildly not ideal (it *requires* you to have a sim/synth mismatch in one particular clause)
<whitequark>
but as it stands, no vendor implements the entirety of SV in synthesis (that wouldn't even make sense), and so you're stuck with N incompatible dialects whether you like it or not, and there isn't even any baseline you can claim conformance to
<az0re>
Since everybody seems to write generators and use Verilog primarily as a language backend, anyway, I wonder if RTLIL will largely supplant it in a few years
<whitequark>
I would hope not (at least, not as RTLIL exists today)
<az0re>
Write your RTL however you want: Bluespec, Chisel, Migen, etc.
<whitequark>
RTLIL is defined by its singular implementation that changes without notice
<az0re>
But those are all more sane HDLs than Verilog IMO
<whitequark>
RTLIL is quite nice, but it simply cannot become an industry standard without negatively impacting... well, everyone
<whitequark>
(Yosys because RTLIL would get effectively frozen, and everyone else who would now lack a formal specification they could adhere to)
<whitequark>
it's like how people keep trying to use LLVM IR for things
<whitequark>
first in PNaCl, then in some shader thing I forget the name of
<az0re>
I dunno if it would get effectively frozen, it would just need to be effectively versioned
<whitequark>
it works for exactly one LLVM release cycle and then you're left with a massive headache and a project you're probably going to deprecate
<whitequark>
why not just use FIRRTL?
<az0re>
Oh, sorry, that's what I meant
<whitequark>
it's conceptually very similar to RTLIL and it is explicitly designed as an interchange format
<az0re>
What Chisel uses on the backend
<whitequark>
oh
<whitequark>
oh yeah I want to add a FIRRTL backend to nMigen some day
<az0re>
So many RTLs and Rs and Is, I got lexically turned around...
<daveshah>
We need a FIRRTL frontend for Yosys at some point
<whitequark>
daveshah: I might write that along with the nMigen backend, possibly
<whitequark>
the current situation where I version-sniff Yosys is not ideal
<daveshah>
Yeah, that would be nice
<whitequark>
I might be stuck doing that to an extent depending on how bad FIRRTL-to-Verilog roundtrip is
<whitequark>
but also maybe not
<sorear>
the best part about SPIR-V is that everyone thinks it's related to RISC-V or vice versa
<daveshah>
Partly because I've been using some of the Chisel cores as large benchmarks, and their Verilog generation on those is pretty slow
<daveshah>
Not massively problematic in the scheme of synthesis of such big thingsi
<daveshah>
But 10 minutes or so for the bigger BOOM configs which seems quite a bit for a fairly simple transformation
<whitequark>
ouch
<az0re>
Oh I can tell you stories about long build times in Chisel... ;)
<whitequark>
i would start reconsidering my choices if i had to wait 10 minutes for *synthesis*
<whitequark>
much less verilog generation
<az0re>
I once had a design that took 40+m to elaborate in a frozen Chisel2 version
<daveshah>
I mean this is a design that will probably take a few hours to go through nextpnr atm
<daveshah>
to get an idea of the order of magnitude
<whitequark>
suddenly, the current nmigen elaboration times start to look not so ba
<whitequark>
*bad
<whitequark>
(i can improve them by an OOM, i think, with some significant but necessary effort)
<az0re>
on an empty dual Xeon E5-2698v4 with all cores being used
<whitequark>
wow
<az0re>
A big part of it for sure was my own code
<whitequark>
was it particularly large?
<az0re>
But at least 15+m was purely in Chisel code
<az0re>
I mean, yeah...
<az0re>
lol
<az0re>
The output Verilog was like 100M IIRC
vidbina_ has quit [Quit: vidbina_]
<whitequark>
I see
<whitequark>
with attributes, comments, etc stripped?
<az0re>
But this was also a very old frozen version of Chisel2 with a very old frozen version of Scala
<whitequark>
(nmigen's verilog output is probably half (*src*))
<az0re>
Nope, raw
<az0re>
But there weren't any attributes emitted at that point
<whitequark>
ah, makes sense
<az0re>
Lots and lots and lots of gunk, useless wires, etc. though
<whitequark>
yes, it's been somehting i've worked on in yosys' write_verilog backend
<daveshah>
Chisel/FIRRTL tends to generate a lot of source comments, ime
<whitequark>
sadly, the sheer complexity of verilog's width/signedness inference has defeated me
<az0re>
And comments, yeah
<daveshah>
similar to the number of nmigen src attributes
<whitequark>
it's tractable, but i spent so long on it, and would have to spend so much more time, that it is really hard to find motivation for it
<az0re>
I wonder if you could do a sort of SAT sweeping approach
<whitequark>
for someone who doesn't write any verilog code basically ever, i know *Way* too much about verilog
<whitequark>
and most of that knowledge is really depressing
<az0re>
Find other equivalent wires, replace all references to the one closest to the driver
<whitequark>
you don't need SAT
<whitequark>
at least for the thing i was doing
<whitequark>
you just need to reconstruct it back to an AST form first
<whitequark>
oh, *that* kind of useless wire
<whitequark>
shouldn't yosys be able to do it already?
<az0re>
Probably
<whitequark>
the share pass
<daveshah>
Yeah, good luck with that on that size design though
<az0re>
opt_share or opt_merge probably get a lot, if not all
<daveshah>
opt_merge should catch most of these cases
<daveshah>
freduce does full SAT equivalence sweeping
<daveshah>
But I've not had much luck with it above about 1000 LUTs
<az0re>
I see
<whitequark>
daveshah: oooh, that should work well for boneless
<daveshah>
it quickly takes way longer than the rest of synthesis
<az0re>
Lots to be tried there
<whitequark>
with the 300 LUT target
<daveshah>
A fast, whitebox aware version is on my TODO list but I don't know when I will get to that
<az0re>
Might be a good idea to try a window, where you look only at nearby (by some metric) cells up to some number
<daveshah>
The trick ABC does is to use random value simulation to find candidate equivalences
<daveshah>
I haven't quite figured out how freduce actually works yet
<az0re>
Yeah, Alan is probably the guy to talk to about this if anyone wants to get serious about improving that code...
<daveshah>
It's based on cones but it doesn't seem to do much to try and reduce the problem
<daveshah>
I plan to reimplement the random value simulation, as it is the most obvious way to filter out things that don't need to go to SAT
<az0re>
Makes sense
<az0re>
I'm sure it's way down on the TODO list though
<daveshah>
Yeah, very much in the "one day" category, nextpnr maintainence and R&D taking up most of my time atm
<az0re>
Yeah that's probably appropriate IMO
<az0re>
Honestly this would be a good semester project for some university student