ChanServ changed the topic of #nmigen to: nMigen hardware description language · code at · logs at
rohitksingh has quit [Ping timeout: 260 seconds]
Degi_ has joined #nmigen
Degi has quit [Ping timeout: 265 seconds]
Degi_ is now known as Degi
<Vinalon> sure, I can try
<whitequark> Degi: awygle: i'm just going to fix it
<whitequark> it shouldn't be broken, and there are tests for it...
<Vinalon> seems like the simulation doesn't finish when I try to make an MCVE.
<Vinalon> oh, cool - I guess I shouldn't bother filing an issue then?
<whitequark> I think the existing issue is enough, yes
<awygle> mk sounds good :)
<Vinalon> on that subject, is it possible to drive Pin outputs from different clock domains based on a runtime setting?
<Vinalon> like, if you have one or more peripherals that use different timings from the rest of the design and a multiplexer that decides where pins are driven from?
<whitequark> Vinalon: you can express such a netlist, but you cannot represent it in many FPGAs
<whitequark> well, not in a way that would reliably work
<Degi> Hm couldnt you use a mux or m.If and m.Else in the comb domain?
<whitequark> for example, ice40 does not inherently provide glitchles muxes
<whitequark> (it is an open question whether you can construct one manually by instantiating LUTs)
<Vinalon> oh, so if you drive a pin with combinatorial logic it might float around a little when it needs to change?
<whitequark> that's another problem with this plan
<whitequark> if you use SB_IO registers, the timings of clock to IO capture or setup are fixed and well known
<whitequark> if you do not, the timings are variable and change from seed to seed
<Vinalon> okay, so maybe the closest I could get would be to drive pins from the fastest clock domain available and make sure that frequency is a multiple of whatever timing is required by the peripherals?
<Vinalon> (I'm thinking about WS2812B/SK6812 LEDs, which use a ~12MHz single-wire signal)
<whitequark> can you explain what are you trying to do that leads you to consider switching between multiple clock somains?
<whitequark> *domains
<Vinalon> well, I just figure that the CPU will have a variable frequency and the pins need to toggle every 12MHz regardless. And all of my iCE40 boards have a 12MHz external oscillator, so I was going to use the 'sync' domain in most cases and a 'sync12' domain when an LED peripheral was selected for a pin
Vinalon has quit [Remote host closed the connection]
Vinalon has joined #nmigen
<whitequark> the CPU will have a variable frequency? can you elaborate?
<Vinalon> well right now, it's running at 6MHz because I'm not very good at FPGA design. And I was hoping to let software configure the clock speed eventually, if that turns out to be possible.
<Vinalon> but I haven't read much about the iCE40 PLL
<whitequark> that might be tricky
<whitequark> cc daveshah, tnt who i think worked with the PLL
<Vinalon> okay - if it's not trivial, I guess I can just use the 12MHz domain for pin outputs for now. I doubt I'll have to worry about faster signals for awhile anyways
<whitequark> the PLL can't output a 6 MHz clock anywya
<zignig> whitequark: (and tpwrules) for the Boneless update. will put some time into getting some stuff working. (now I have some unexpected free time ;) )
<Vinalon> oh, cool - thanks!
<zignig> *+thanks for the
Vinalon has quit [Ping timeout: 256 seconds]
Vinalon has joined #nmigen
____ has joined #nmigen
FFY00 has quit [Remote host closed the connection]
FFY00 has joined #nmigen
rohitksingh has joined #nmigen
thinknok has joined #nmigen
nengel is now known as attie
Asu has joined #nmigen
thinknok has quit [Ping timeout: 264 seconds]
Vinalon has quit [Ping timeout: 256 seconds]
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen] whitequark 7ba362a - Travis: upgrade to bionic.
<_whitenotifier-3> [nmigen] Failure. 82.36% (+-0.34%) compared to 91d6e4b -
<_whitenotifier-3> [nmigen] Success. Coverage not affected when comparing 91d6e4b...7ba362a -
<_whitenotifier-3> [nmigen] Failure. 82.64% (+-0.06%) compared to 91d6e4b -
<_whitenotifier-3> [nmigen] Success. 82.69% (+0.00%) compared to 91d6e4b -
thinknok has joined #nmigen
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen] whitequark 8b13743 - Travis: require tests to pass on pypy3.
<_whitenotifier-3> [nmigen] whitequark closed issue #147: pypy3 support tracking issue -
<_whitenotifier-3> [nmigen] Failure. 82.64% (+-0.06%) compared to 7ba362a -
<_whitenotifier-3> [nmigen] Success. Coverage not affected when comparing 7ba362a...8b13743 -
<_whitenotifier-3> [nmigen] Success. 82.69% (+0.00%) compared to 7ba362a -
<MadHacker> I just made a stupid mistake that I feel should maaaybe have some sort of catch somewhere in nmigen. Signal(0) doesn't seem to raise any error. :D
<whitequark> that's deliberate
<whitequark> 0-width signals and values are explicitly supported so that you can write generic code more easily
<whitequark> in fact you can see from the issues like that people *do* find them useful
<whitequark> (that one isn't fixed, but 0-width values should work fine in general)
<MadHacker> Hm, I can sort of see it for a zero-width address, I guess. It feels like a thing where the mistake is more likely than the practical use, but I take the point.
<MadHacker> I'll chalk that one up to experience and just be more careful. :)
<whitequark> in general, nmigen does not try to aggressively catch mistakes where a netlist "isn't quite right"
<whitequark> there are some languages with really aggressive code linters, nmigen is not currently one of them
<whitequark> which i think is fine, my opinions might strongly differ from yours
<MadHacker> Aye, I'm fine with the general principle. That one felt like a classic newbie mistake of the "probably worth catching even on a relaxed language" variety, but I can see the logic.
<whitequark> one of the most important principles of nmigen is enabling flexible generic code, so 0-width things are quite important in my vie
<whitequark> *view
futarisIRCcloud has quit [Quit: Connection closed for inactivity]
<ZirconiumX> whitequark: Purely by accident, I think rotate_left/rotate_right do the right thing when you pass negative values to them.
<ZirconiumX> Though I think they break when out of range
<tnt> The lack of 0 width signal in verilog is the most annoying thing I encounter daily ...
<tnt> so +1 on them being an essential feature
<_whitenotifier-3> [nmigen] ZirconiumX synchronize pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<whitequark> tnt: amusingly i hit an issue where write_verilog would actually not correctly emit 0-width signals
<whitequark> in yosys
<whitequark> and turns out the officially permitted way, explicitly listed in the spec, is to use {0{1'b0}}
<MadHacker> Cast a width 1 zero to width 0?
<MadHacker> That's pretty ugly.
<whitequark> no, verilog 2001 does not have casts
<whitequark> (wtf?!)
<whitequark> that code is a repeat expression
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<MadHacker> Oh wait what. Sorry. WTF.
<whitequark> they explicitly tell you in 1364-2001 to repeat any value 0 time whenever you need a 0-bit value
<tnt> whitequark: how do you deven define a zero width wire ? wire [-1:0] ?
<whitequark> you can't do it with a wire, yeah
<whitequark> but you can have a 0-bit value at least
<whitequark> (or else nmigen would have a lot more trouble emitting verilog)
<tnt> right ... but you can't have 0-bit slices either.
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] whitequark reviewed pull request #352 commit -
<_whitenotifier-3> [nmigen] whitequark reviewed pull request #352 commit -
<_whitenotifier-3> [nmigen] whitequark reviewed pull request #352 commit -
<_whitenotifier-3> [nmigen] ZirconiumX synchronize pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
Vinalon has joined #nmigen
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] whitequark reviewed pull request #352 commit -
<_whitenotifier-3> [nmigen] whitequark reviewed pull request #352 commit -
<_whitenotifier-3> [nmigen] ZirconiumX synchronize pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<ZirconiumX> wq: I was *really* tempted to try sneaking in a "meow" comment next to the Cats.
<whitequark> I would not have flagged that :p
<ZirconiumX> ...Hmmm...
<ZirconiumX> >.>
<_whitenotifier-3> [nmigen] ZirconiumX synchronize pull request #352: Add rotate by constant -
<ZirconiumX> <.<
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<Sarayan> You mean <<< and >>> of course
<ZirconiumX> If Python let you define arbitrary operators, then sure :P
<Sarayan> huhuhu
<whitequark> they sort of discussed it before
<whitequark> not completely arbitrary
<whitequark> but there's @
<Sarayan> python is bignum by default, right? So rotation would be severely undefined
<Sarayan> Not sure why C/C++ still doesn't have it though
<whitequark> because they're high level languages :p
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<Sarayan> higher level than java?
<Degi> Hmm why is it called right and left shift anyways...
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<Degi> Like why arent things just MSB and LSB oriented
<whitequark> because programming is clearly only for anglophones
<whitequark> i'm pretty sure right/left shift/rotate terminology breaks down for RTL languages
<Sarayan> huh?
* ZirconiumX tries to think of an RTL number system
<Sarayan> ZX: Roman?
<Sarayan> ah no
<ZirconiumX> e.g. 1000 << 1 is a left shift because the digits are moving left
<Sarayan> and it's not a number system anyway
<ZirconiumX> The Ancient Egyptians probably had one :P
<whitequark> hm, actually i'm wrong here
<whitequark> both hebrew and arabic numbers are written in the same direction
<whitequark> it's still a bad convention purely because a significant number of people can't think in terms of right/left
<whitequark> *same direction as in english
<Degi> Well doesnt english use arabic numbers
<ZirconiumX> Mayan appears to be written vertically, so I guess it breaks down there had the Ancient Mayans survived
<Degi> What would a right/left shift on roman numerals even look like
<ZirconiumX> Yes, it's the Arabic numeral system
<whitequark> Degi: yeah, but the key part is they were imported complete with the *writing order*
<whitequark> which i didn't realize before
<whitequark> ZirconiumX: CI fails on your PR
<MadHacker> Yeah, our written numbers are big-endian; even when written longhand like "five million two hundred thousand and seven". :)
<Degi> Hm I have a rotator implementation working with signals, should I PR that too?
<whitequark> what do you mean by "working with signals"?
<ZirconiumX> Degi: the tricky bit is rotates by non-powers-of-two
<Degi> The rotation amount is a signal
<ZirconiumX> Which by the definition of rotate require a combinational modulus
<Degi> Huh?
<Sarayan> Degi: tr '[IXC]' '[XCM]' kinda? :-)
<Degi> What is IXC and XCM?
<Sarayan> 1 10 100 1000
<ZirconiumX> Roman numerals
<Degi> Ah yes
<whitequark> ZirconiumX: now that I think about it, we could make the legalizer emit Degi's code
<whitequark> but I'm not sure if we should
<Degi> I kinda feel like a more elegant implementation than a for loop of cases should be possible
<Degi> Wonder how that scales like. I guess len(datain)^2 number of gates
<whitequark> you can do it for rotates of power-of-2 sized signals
<whitequark> just chop off the top bits from the rotate amount
<MadHacker> You could do a combinatorial set of all the rotation positions chained in sequence, and then use the case to output one of them, in the style of the classic barrel shifter?
<MadHacker> But eats a lot of area anyway.
<Degi> Yes I think thats what my code does
<whitequark> but not for rotates of NPOT sized signals where you have to either do a huge mux, or a small mux and a combinatorial modulo
<whitequark> both of which are awful
<whitequark> I wanted to avoid having pathological performance cliffs so that's why I asked ZirconiumX to avoid rotates by a variable
<ZirconiumX> Somewhat less awful on LUT6 architectures, but still not great
<Degi> Hm I think my code makes a mux with size of len(datain)
<Sarayan> multiple layers depending on the prime number decomposition of the size?
<whitequark> Sarayan: sure, and you get to maintain and support that
<Sarayan> that's reaching the point where you want the designer to do the thing he wants
<Degi> Hm so my implementation was for rotating 8 bit signals (though 12 bits and 14 bits might be needed too)
<Sarayan> and not have to maintain a generic insanity
<whitequark> Degi: your implementation is wrong for some inputs
<whitequark> specifically, for the cases where self.rotation > len(self.datain)
<_whitenotifier-3> [nmigen] ZirconiumX synchronize pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<ZirconiumX> Anyway, while we wait for a consensus here
<whitequark> if that's fixed the mux will be 2**len(rotation) sized
<Degi> Hm I think we can limit to len(datain)^2
<Degi> Oh I forgot that because in my case the rotation signal is 3 bits long and datain 8 bits long, but yes that should be implemented too
<whitequark> this gets pretty complex pretty quickly
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<whitequark> which is why I think for now rotates by a constant amounts are just fine
<_whitenotifier-3> [nmigen] codecov[bot] edited a comment on pull request #352: Add rotate by constant -
<_whitenotifier-3> [nmigen] whitequark closed issue #339: Add rotate left/right by constant amount -
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±3]
<_whitenotifier-3> [nmigen/nmigen] ZirconiumX 06c45c9 - hdl.ast: add Value.{rotate_left,rotate_right}.
<_whitenotifier-3> [nmigen] whitequark closed pull request #352: Add rotate by constant -
<ZirconiumX> Hurray
<_whitenotifier-3> [nmigen] Failure. 82.67% (+-0.03%) compared to 8b13743 -
<_whitenotifier-3> [nmigen] Success. 100.00% of diff hit (target 82.69%) -
<ZirconiumX> Next on my endless to-do list is to file a Yosys enhancement request to have it break up $add$sub cells where it can prove it will or won't carry
<ZirconiumX> *$add/$sub
<_whitenotifier-3> [nmigen] Success. 82.72% (+0.02%) compared to 8b13743 -
<ZirconiumX> wq: What would you call a generalised-shift (shift direction based on rhs side)? x.shift(foo)?
<whitequark> ZirconiumX: what's the difference between that operation and one of the existing shifts?
<whitequark> they support negative rhs already
<ZirconiumX> I thought they didn't...
<whitequark> hm
<whitequark> oh, sorry, *shifts*
<whitequark> not rotates
<whitequark> ZirconiumX: instead of adding a .shift operation which *actually* implements a left shift, which would be confusing, why won't we relax the restrictions on shift amounts?
<whitequark> this will require patching pysim and rtlil backends though
<ZirconiumX> It depends
<ZirconiumX> We can very slightly relax it to allow negative shifts by constexprs
<ZirconiumX> And emit the inverse operation in that case
<whitequark> it seems strange to have varshifts as << >> and constshifts as .shift_left .shift_right
<whitequark> ah, I see what you mean
<whitequark> hmm
<whitequark> so one reason << >> should not be implementing shifts with a negative amount is because Python doesn't have those
<ZirconiumX> Does that mean Python would error out in that case, or does it still call the relevant operator overload function?
<whitequark> try it
<whitequark> oh
<whitequark> no, it calls into nmigen
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±2]
<_whitenotifier-3> [nmigen/nmigen] whitequark b44870e - Clarify a few comments. NFC.
<ZirconiumX> Mmm, I can see this being a bit tricky
<ZirconiumX> e.g. C(1) << -3 does C(1) >> 3, but 1 << -3 is a Python error that we can't catch
<whitequark> yep
<whitequark> I am fine with having separate .shift_left and .rotate_left which do support negative amounts
<whitequark> er
<whitequark> .shift_left and .shift_right
<Sarayan> can you shift by a non-constant?
<whitequark> the rule of thumb here is that << >> follows Python semantics and is "cheap", whereas .shift_* and .rotate_* follow extended semantics and might be "expensive"
<whitequark> Sarayan: yes
<whitequark> ZirconiumX: so, .shift_* could be free to use a mux for signed shift amounts
<_whitenotifier-3> [nmigen] Failure. 82.67% (+-0.06%) compared to 06c45c9 -
<_whitenotifier-3> [nmigen] Success. Coverage not affected when comparing 06c45c9...b44870e -
<whitequark> and .rotate_* could be free to get legalized in the backend
<ZirconiumX> That sounds like a good design philosophy
<_whitenotifier-3> [nmigen] Success. 82.72% (+0.00%) compared to 06c45c9 -
<ZirconiumX> Though equally I feel like I'd worry about corner cases in something like that
<ZirconiumX> e.g. an accidental off-by-one in some generic code turning a cheap 64-bit rotate into an expensive 65-bit rotate
<ZirconiumX> Well, you kinda get the idea
<whitequark> yes
<whitequark> in that case you should probably have an assert in that generic code
<Sarayan> Could you have warnings for things like that and easy way to shut off specific warnings in specific places?
<ZirconiumX> I'd be okay with that, I think
<ZirconiumX> ExpensiveOperationWarning or something
<ZirconiumX> Such as the rotate producing a combinational modulus or whatever
<whitequark> we shouldn't ever emit a combinatorial modulo i think
<whitequark> just a huge mux
<whitequark> that one is still expensive, but it's predictably expensive
<ZirconiumX> Okay, fair
<ZirconiumX> Would you consider something like popcount and lzcnt/tzcnt?
<MadHacker> Yep, noone will be surprised by that output. A weird combinatorial thing some of the time would be unexpected IMHO.
<whitequark> the problem is that you really want these warnings to be emitted in the optimizer, i.e. yosys
<ZirconiumX> I'm not actually sure if Quartus would even accept a combinational modulo
<whitequark> popcnt and ff1/fl1 seem like obvious additions, but it's not obvious how to lower them in a good way
<ZirconiumX> I see two approaches, really
<ZirconiumX> One which returns, say, ff1 in one-hot form (which is the kind of thing I need), and one which returns ff1 in bit-index form
<whitequark> one-hot ff1 is not something I think is useful as an operator
<whitequark> especially since it's just x & -x anyway
<ZirconiumX> Perhaps, but I think that could equally be applied to the index form. FPUs would need it for normalising results, maybe, and CPUs could use it as an instruction
<whitequark> then let's not have it at all
<whitequark> as an operator anyway
<ZirconiumX> I actually ditched `x & -x` in favour of essentially an unrolled recurrence relation
<whitequark> my main concern here is the implementation
<whitequark> for rotates, there is essentially one way to reasonably implement them, and the operations themselves are primitive
<whitequark> for popcnt, that is not the case
<whitequark> I think such things are best left to libraries, at least at first
<ZirconiumX> I see your point
<whitequark> (well, const rotates aren't quite primitive in nmigen, since you implemented them in terms of slices and concatenations. but they are a common operation on bit vectors, and if we added variable rotates, they'd have to be legalized, too, making them a true primitive)
<ZirconiumX> I'd be in favour of making rotate a true primitive, but it perhaps needs some thought first
<Sarayan> do you have hardware that does it natively?
<whitequark> yosys doesn't have a rotate cell anyways
<ZirconiumX> daveshah: Can the CrossLink-NX ALUs do a rotate operation?
<daveshah> Yes they can
<daveshah> as a non-standard extension
<daveshah> technically, is this the first tapeout of (a small part of) claire's bitmanip?
<ZirconiumX> I think so
Vinalon_ has joined #nmigen
<daveshah> I don't think they use the defined encoding, but I don't think the bitmanip encodings are/were fixed anyway
<ZirconiumX> Sarayan: Well, there's your answer
<Sarayan> as in "not really"?
<ZirconiumX> Last time I checked "yes there is hardware that does it natively" doesn't mean "not really"
<daveshah> It's the kind of thing that might be a primitive in a CGRA
Vinalon has quit [Ping timeout: 260 seconds]
<ZirconiumX> CGRA?
<Sarayan> I'm not sure what "as a non-standard extension" means
<daveshah> coarse-grain reconfigurable array
<whitequark> whether rotates are primitive or not in nmigen has quite little to do with whether any hardware has them
<daveshah> Sarayan: the CrossLink NX ALUs implement the RISC-V ALU instruction set
<whitequark> an nmigen primitive is something the nmigen backend needs to be aware of, for whatever reason
<daveshah> plus rotates and some kind of fixed-point multiply
<Sarayan> wq: if it has to be lowered no matter what, there's less interest in having it primitive compared to library
<daveshah> Given that there is only one of these blocks in the currently available parts, it probably has better uses than a single rotate
<Sarayan> not zero interest, but less interest
<whitequark> Sarayan: quite the opposite
<Sarayan> really?
<whitequark> well, consider Array
<whitequark> it has to be lowered no matter what, yet if you tried to implement it in a library, it would be a rather unfortunate experience
<Sarayan> Array is a structure, not an operatiob
<Sarayan> Array is a structure, not an operation
<Sarayan> that's different, I think
<whitequark> array indexing is an operator
<whitequark> let me try to explain it in another way
<whitequark> if implemented as a primitive, `x.rotate_left(y)` will always return an expression and that's it
<whitequark> so you can use it in any expression context. you don't have to add a submodule
<Sarayan> 'kay
<whitequark> this includes the case where it has to be expanded into a switch!
<Sarayan> librairies are required to be submodules?
<ZirconiumX> wq: One thing which came to mind about rotate is that since presently Yosys does not inline cells, it might not be obvious where the rotate is taking place in output Verilog
<whitequark> if you implement rotate_left() in a library and sometimes it expands into a switch, you have a few options
<whitequark> 1. make it a submodule
<whitequark> 2. make it *take* a module
<whitequark> 3. make it emit a massive Mux tree
<whitequark> none of those are very good
<Sarayan> hmmm
<Sarayan> interesting
<whitequark> ZirconiumX: sure, but the solution to that is make it inline cells
<ZirconiumX> Of course
<MadHacker> Anonymous secret self-adding submodules?
<Sarayan> MadHacker: Can I have some whipped cream on top?
<MadHacker> (i.e. 2 without telling you about it)
<whitequark> MadHacker: yes, that is an option
<Sarayan> I tend to say that if something is big, it should be rather visible
<whitequark> whether it is a good option, that is not clear
<Sarayan> (of course C++ breaks that left and right, but heh :-)
<whitequark> it's certainly not a good option right now because the hierarchy handling is still a bit wonky with respect to clock and reset signals
<whitequark> and because Fragment.inline is still not entirely functional
<whitequark> you know what we need? randomized testing of the backends against each other
<Sarayan> two come in, one comes out?
<whitequark> lol
<whitequark> more or less
<whitequark> generate random netlist, see if pysim and yosys thinks it evaluates to the same thing
<whitequark> i bet you it will find dozens of bugs ... everywhere
<whitequark> add cxxrtl to the mix, now you can select 2 of 3 and shoot the one that is in minority
<whitequark> (don't do this; i wrote 2 of 3 so the majority might well be wrong)
<Sarayan> how do you test "evaluates to the same thing"?
<whitequark> what do you mean?
<Sarayan> "see if pysim and yosys thinks it evaluates to the same thing"
<whitequark> sure
<Sarayan> you do what, random deterministic inputs?
<whitequark> yup
<Sarayan> then that wpuld be pysim vs. cxxrtl?
<Sarayan> yosys by itself doesn't simulate anything, right?
<whitequark> yosys> help eval
<whitequark> try this
<Sarayan> oh
<Sarayan> wow
<daveshah> There's a "sim" command too
<daveshah> for clocked simulations, eval is probably more useful here
<whitequark> oh whoa that's useful
<Sarayan> I didn't realize yosys had everything including the kitchen sink
<whitequark> the problem with eval is readback
<whitequark> i'll have to parse its truth tables or something
<whitequark> it might be easier to read the VCDs
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen] whitequark edd2bb2 - back.rtlil: fix legalization of Part() with stride.
<whitequark> awygle: here, word_select works properly now
<Degi> Nice
<Degi> Now my mcve works fine
<Degi> Wait nah the bit_select still amkes an error
<whitequark> yes
<whitequark> word_select in your MCVE now actually works
<whitequark> instead of just doing nothing
<Degi> Oh wait I need to update the nmigen lol
<whitequark> bit_select is still broken though
<Degi> Cant you use word select with width 1 as bit select
<whitequark> yes
<Degi> Oh neat pip can install straight from git
<whitequark> yep
<Degi> I see, before there was switch \counter with no statements and now it works properly. Thanks!
<whitequark> let me see what is happening with .bit_select here...
<whitequark> Degi: oh I see
<whitequark> what happens is your bit_select expression has a few cases where it picks up some high order bits from the data, and then some bits that don't exist in data
<Degi> Huhh?
<whitequark> hm, hang on
<whitequark> that's what the backend sees, but it shouldn't
<_whitenotifier-3> [nmigen] Failure. 82.39% (+-0.34%) compared to b44870e -
<_whitenotifier-3> [nmigen] Success. Coverage not affected when comparing b44870e...edd2bb2 -
<Degi> I mean in the only difference between bit_select and word_select is that in one stride is 1 and the other stride is width
<whitequark> ah, yes
<whitequark> len(counter * 4) == 4
<whitequark> because of how length calculation is done for multiplication
<_whitenotifier-3> [nmigen] Failure. 82.44% (+-0.29%) compared to b44870e -
<_whitenotifier-3> [nmigen] Success. 82.72% (+0.00%) compared to b44870e -
<Degi> Oooh it should be 2 right?
<Degi> Yeah that makes sense why it says the thing with 9 and 8 bits
<whitequark> it could be 0 or 4, so 3
<Sarayan> "it could be 0 or 4, so 3"
<whitequark> it's 4 because the length calculation is conservative
<Sarayan> beautiful sentence
<Degi> Hm does it just add bit lengths? Like signal is 1 bit and 4 is 3 bits and together thats 4 bits?
<whitequark> yes
<Sarayan> hmmm, what's the number of bits of (2**n-1) * (2**m-1), interesting question
<awygle> Wow, backlog
<Degi> Hmm now I get yosys -V returned 127 because libffi is missing despite it being there
<Sarayan> it's 2**(n+m)+1 - 2**n - 2**m
<Degi> And then log_2 of that?
<daveshah> Degi: I just had to rebuild Yosys because of that libffi issue
<daveshah> I guess a recent update
<Degi> k
<Degi> Ill do that
<Sarayan> hopefully it's possible to do better than that
<Degi> Multiplication of a*b should take log_2(a)+log_2(b) bits right
<Sarayan> you don't know the value, so you need the max
<Sarayan> and the max is not a power of two, obviously
<Degi> Then bits(a)+bits(b) I thibk
<Sarayan> well, it feels like bits(a)==1 or bits(b)==1 is a special case
Vinalon has joined #nmigen
<Degi> Hm yeah 3*3=9 etc
<Sarayan> because if say n=1 then 2**(1+m)+1-2**1-2**m = 2*(2**m) + 1 - 2 - 2**m = 2**m-1
<Degi> Hm but bits(a)=1 and bits(b)=3 for line (0, 1)*4 would yield 4 bits despite being storeable in 3 bits
<Sarayan> so m bits and not m+1
Vinalon_ has quit [Ping timeout: 265 seconds]
<Sarayan> but that only happens for on of the bit counts being 1
<Degi> Maybe the code could have a case where it subtracts 1 from the calculation if one of the bit counts is 1
<Sarayan> wq: You could special case when one of the sides is single-bit unsigned, because it's the only special case but an obvious one too
<Sarayan> in addition, it's a n-way and too, no reason to do a multiplication
<Degi> Maybe we can use ceil(log_2(a_max))?
<Degi> That way a 1 bit value yields 0
<Sarayan> why go with complicated functions when testing 1 is exactly as correct?
<Degi> Hm yeah
<whitequark> Sarayan: maybe, unclear if the added complexity is worth it
<whitequark> or rather, i'd rather have randomized testing first and special cases in some of the most obscure corners of nmigen second
<Degi> Hmm on the other hand, if you have bits(a)=2 and bits(b)=3 and a_max=2 and b_max=4 that gives a_max*b_max=8 which fits into 4 bits while (2^2-1)*(2^3-1)=21 which needs 5 bits. That would lead to the same kind of error in bit_select
<Sarayan> well, if you don't special-case it w.r.t generation, it would be nice to have a "stupidly expensive operation" warning
<whitequark> folks
<whitequark> the problem with bit_select is a simple backend bug
<Degi> Hm okay
<whitequark> please stop overthinking width inference, which is completely irrelevant to the bug
<whitequark> you'll have it if you ditch the multiply and widen the counter just as well
<whitequark> try it
<whitequark> counter = Select(4)
<whitequark> *Signal(4)
<awygle> Okay caught up. Busy night lol
<awygle> Thanks for fixing word select wq!
<Degi> Hm how would you handle the counter being out of bounds?
<whitequark> extend the value
<Degi> The input value?
<whitequark> yeah
<whitequark> Degi: and fixed
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen] whitequark 814ffde - back.rtlil: fix expansion of Part() for partial dummy writes.
<_whitenotifier-3> [nmigen] whitequark closed issue #351: bit_select and word_select behave differently -
<whitequark> it turns out the problem was actually related to using bit_select on LHS with partially out of bounds part selected
<whitequark> which is a particularly weird edge case
<whitequark> that i already handled for fully out of bound parts but not partial ones
<_whitenotifier-3> [nmigen] Failure. 82.63% (+-0.10%) compared to edd2bb2 -
<_whitenotifier-3> [nmigen] Failure. 0.00% of diff hit (target 82.72%) -
<_whitenotifier-3> [nmigen] Failure. 82.68% (+-0.05%) compared to edd2bb2 -
<Degi> Heh
<whitequark> see, this is the kind of thing fuzzers excel at discovering
<Degi> Oh no I have the official nmigen-boards now instead of my hacked together version and now need to write my code properly lol
<whitequark> even not comparing the results would probably discover a lot of issues
<Degi> whitequark: I added a PR to nmigen-boards for the connectors
<_whitenotifier-3> [nmigen-boards] x44203 opened pull request #59: Add connectors -
<whitequark> have you checked them for correctness, as the TODO requests?
<Degi> Hmm I typed them it, but yeah I can do that
<whitequark> proofreading is often helpful IME
<ZirconiumX> ls
<ZirconiumX> ...
<ZirconiumX> Sorry
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen] whitequark 792f35a - back.rtlil: refuse to create extremely large wires.
<_whitenotifier-3> [nmigen] whitequark closed issue #341: nMigen should avoid emitting very large wires that cause issues in Yosys -
<awygle> On a roll
<_whitenotifier-3> [nmigen] whitequark commented on issue #341: nMigen should avoid emitting very large wires that cause issues in Yosys -
<Degi> Huh there is an error in the datasheet
<_whitenotifier-3> [nmigen] Failure. 82.61% (+-0.08%) compared to 814ffde -
<_whitenotifier-3> [nmigen] Failure. 50.00% of diff hit (target 82.68%) -
<Degi> It says that J_39 29 is named E7 but not connected to the FPGA but in the schematics its connected to ball E7
<whitequark> hm
<_whitenotifier-3> [nmigen] Failure. 82.66% (+-0.03%) compared to 814ffde -
<Degi> I found a mistake actually, can I edit the file?
<whitequark> sure
<Degi> Hm for some reason tehre's L20/M20 besides each other which are a differential pair, but the datasheet of the eval board doesnt say anything about that. I guess that's because of routing differences?
<_whitenotifier-3> [nmigen-boards] x44203 synchronize pull request #59: Add connectors -
<Degi> Should be done
<whitequark> yes, might not have been routed as a diffpair
<_whitenotifier-3> [nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen] whitequark ee73d39 - back.rtlil: don't emit connections to zero width ports.
<_whitenotifier-3> [nmigen] whitequark closed issue #335: Records with zero-width fields used as module ports break Yosys -
<whitequark> awygle: bunch of easy bugs
<_whitenotifier-3> [nmigen] Failure. 82.31% (+-0.36%) compared to 792f35a -
<_whitenotifier-3> [nmigen] Failure. 50.00% of diff hit (target 82.66%) -
<_whitenotifier-3> [nmigen] Failure. 82.59% (+-0.07%) compared to 792f35a -
<_whitenotifier-3> [nmigen] Failure. 82.64% (+-0.02%) compared to 792f35a -
<whitequark> hm. i want to see notifications from travis, and i do not want to see notifications from coveralls
<whitequark> instead we have the exact opposite situation
<whitequark> i hate it
thinknok has quit [Ping timeout: 265 seconds]
Vinalon has quit [Ping timeout: 265 seconds]
<awygle> I agree
<Degi> Can you accept the pr on nmigen-boards? It should be all correct. Thanks!
<_whitenotifier-3> [nmigen-boards] whitequark closed pull request #59: Add connectors -
<_whitenotifier-3> [nmigen/nmigen-boards] whitequark pushed 1 commit to master [+0/-0/±1]
<_whitenotifier-3> [nmigen/nmigen-boards] x44203 bca5096 - ecp5_5g_evn: add connectors.
<awygle> whitequark: do you have any issues in the nmigen "core" that you'd consider good beginner issues? (i should have asked this before you closed half a dozen issues obviously but that's what reminded me to ask)
<whitequark> awygle: hmmm
<awygle> i'd like to learn a bit about how nmigen is implemented, and also be able to help out with simple stuff
<whitequark> define simple
<whitequark> there's the AsyncFIFO stuff for example
<whitequark> or do you mean the compiler itself?
<awygle> i did mean the compiler, or the backend. stuff that's not stdlib basically (not that i won't contribute to the stdlib but i feel like i mostly understand _how_ to do that)
<whitequark> right, hm
<whitequark> you could deprecate Record.connect
<whitequark> with the added element that you also need to deprecate the directions
<whitequark> (which you'll have to do in the layout constructor)
<whitequark> or still too much of a library?
<Degi> Thanks!
<awygle> admittedly not really what i had in mind but i can certainly do it (a goal here is to be helpful to you lol). do we want to deprecate it without a replacement? i thought we were waiting for a better alternative
<whitequark> deprecate without an alternative, i think it is broken in principle
<whitequark> it's kind of unfortunate because a lot of work went into it
<whitequark> but it's just bad design
<awygle> yeah :/ bummer
<awygle> ok i can do that
<awygle> i may also take a swing at #320, which may or may not be simple but i suppose i'll find out relatively quickly
<awygle> i also said i'd do something the other day... _checks logs_
<whitequark> that one is really tricky i think
<whitequark> but you can at least find out the root cause
<whitequark> that would be helpful
<awygle> copy
<whitequark> the fragment transformation code will go away, but use-def graph construction code never will, so this work shouldn't be wasted
<whitequark> awygle: oh, you can implement #254
<whitequark> with the caveat that the yosys code should be changed too
<whitequark> but it's not too hard to change the RTLIL backend to emit whatever you want
<awygle> ah
<awygle> oh it was asymmetric memories
* awygle starts a todo list...
Vinalon has joined #nmigen
thinknok has joined #nmigen
<awygle> okay so in order i'm planning to finish #181 (FIFO reset), #342 (deprecate Connect), #254 (Yosys enum stuff), #217 (FIFO metastability), and then asymmetric memories (no bug)
<awygle> i make no timeline promises, as ever :)
thinknok has quit [Ping timeout: 265 seconds]
chipmuenk has joined #nmigen
proteus-guy has quit [Ping timeout: 264 seconds]
thinknok has joined #nmigen
____ has quit [Quit: Nettalk6 -]
thinknok has quit [Ping timeout: 265 seconds]
chipmuenk has quit [Quit: chipmuenk]
peepsalot has quit [Ping timeout: 258 seconds]
<awygle> whitequark: can i get a hand with something? i'm getting this error when requesting an SRAMResource:
<awygle> ` Resource component sram_0__d uses physical pin M14, but it is already used by resource component sram_0__d that was requested earlier`
<awygle> i'm only calling platform.request once in elaborate() of my testbench
<awygle> but i am assigning from the result multiple times, e.g. `sram_pins = platform.request("sram"); address_o = sram_pins.a; data_io = sram_pins.d`
<awygle> is that likely to be the problem?
<awygle> no never mind i'm an idiot, i listed M14 twice somehow
Asuu has joined #nmigen
Asu has quit [Ping timeout: 256 seconds]
Asuu has quit [Remote host closed the connection]
<Degi> Is it possible to make aliases for balls?
futarisIRCcloud has joined #nmigen
peepsalot has joined #nmigen