<_whitenotifier-f>
[nmigen] whitequark commented on issue #423: Emit a warning when an Assign expression is not added to a domain - https://git.io/Jkvz1
Degi has quit [Ping timeout: 240 seconds]
Degi has joined #nmigen
noknok has quit [Remote host closed the connection]
noknok has joined #nmigen
<awygle>
whitequark: i've continued thinking about the Past thing and what bothers me about Past(domain="x") is that it doesn't make a lot of sense when you think about what "the past value of a signal" is/means. what you're actually saying is "sample this signal in domain x", but if the signal isn't in the control set of the ClockDomain associated with the name "x", that's not "the past value of the signal".
<awygle>
not to put words in your... uh, brain, i guess, but it seems to me like you're mentally snapping the pointer of "$past in SystemVerilog is syntactic sugar for a chain of FFs" and reasoning from that point
<whitequark>
awygle: you're pretty close to truth, actually
<whitequark>
and i now see where our disagreement lies
<awygle>
when i think about the problem you're raising of "how do i write a formal spec that works across modules?", the missing feature seems to me to be the lack of a way to Assert that "this set of signals is in the same control set"
<whitequark>
you see Past as retrieving a (runtime) property of a signal. i see Past as applying a transformation to an expression
<awygle>
yes, exactly
<awygle>
which is why you started talking about type theory and i mentally stalled out on it
<whitequark>
currently, it's true that Past requires a signal as an argument
<whitequark>
but... that's mostly an artifact of how I chose to implement it
<whitequark>
re "what you're actually saying is "sample this signal in domain x"
<whitequark>
Past() quite literally is an alias of Sample()
<awygle>
i did know that, yeah
<whitequark>
re "same control set": using #6 with instances will require coming up with a way to describe control sets of their ports
<whitequark>
something similar could apply to module interfaces
<mithro>
whitequark: It also supports just using YoWasp if you don't want to use the netlistsvg diagrams
<whitequark>
mithro: hmmm I very much do want to use netlistsvg
<whitequark>
in fact that was my whole motivation for it
<whitequark>
and our CI uses yowasp
<whitequark>
is there any fundamental problem with using yowasp and netlistsvg?
<mithro>
whitequark: No, it's more that netlistsvg requires npm
<whitequark>
oh that's fine, I can just install that
<whitequark>
awygle: re interpretation of $past: I think the one you are proposing has merit, conceptually. I have two concerns about it.
<mithro>
We do have a conda environment.yml which will get you Yosys + netlistsvg that works on readthedocs
<whitequark>
I refuse to touch conda voluntarily.
<mithro>
@whitequark If you know another way to get npm/netlistsvg on readthedocs, it should also work
<whitequark>
hm, we don't use rtd
<mithro>
whitequark: Then you are probably fine then
<whitequark>
github actions + pages
<mithro>
whitequark: I really like that readthedocs now builds preview docs on pull requests
<mithro>
We do need to find some people who want to work on improving netlistsvg
<cr1901_modern>
awygle: I drew it out and agree w/ you, but with one caveat:
<cr1901_modern>
If you have clock domain "x" and "y" and Signal "foo" clocked by "x", then "y" _can_ see a past value of "foo" if "y" is running synchronous to "x" and an integer multiple freq of "x".
<awygle>
i did think about phase-locked clocks a bit
<cr1901_modern>
"foo"'s value will never change during an given clock cycle of "y" in that case, so Past("foo", domain="y") is valid there
<awygle>
and i don't have a good solution
<cr1901_modern>
But that's the only case I can think of offhand
<awygle>
but i think that's not super common, and there's always the "do it manually" fallback
<awygle>
(that's also relevant for #6, btw)
<cr1901_modern>
By "concept of past", do we agree that a clock domain has a "concept of past value" if the during any given clock period, the signal does not change in between the active edge (loosely)?
<cr1901_modern>
(Maybe I worded this badly)
<awygle>
i can accept that as a valid definition but i don't think the added capability it gives is worth the added confusion over just saying "the past value of a signal is only defined in its own clock domain/control set"
<whitequark>
awygle: first: I'm quite comfortable with developing a HDL and I know I can do much better than Verilog. however, I don't really want to be responsible for developing a brand new formal specification language and I'm definitely unconvinced I can do better than SVA (or rather SVA's underlying formalism)
<awygle>
understandable
<cr1901_modern>
That's fair... it's mainly for my own peace of mind
<cr1901_modern>
Doesn't help the confusion that SVA has rules to determine what clock you mean if you omit the clocking_event parameter
<whitequark>
second: while the model you're proposing definitely has merit on its own, I don't think it's a good fit for nMigen. nMigen is a simple language whose strength is that has a few simple primitives that behave in a very regular way and therefore can be easily combined to express complicated concepts. I think that introducing a complex global analysis that determines the behavior of Past() and nothing else
<whitequark>
in the language when we can use a trivially simple local transformation that is essentially equally expressive would be a good part of its design.
<awygle>
assuming "i don't think" for that last sentence
<whitequark>
yeah
<whitequark>
can't see the whole sentence in irssi :s
<awygle>
i feel that SVA is quite strongly coupled to SystemVerilog's syntax+semantics (fair enough, it's in the name), and trying to hew too closely to it may cause more harm than good
<whitequark>
yes. that's evident in #534.
* cr1901_modern
nods
<awygle>
and i further feel that the aforementioned "complex global process" is 90%+ required for #6, and Past's behavior is a small delta on top of that
<awygle>
though of course you'd know better than me
<whitequark>
I'm not talking about the implementation
<whitequark>
I'm talking about the mental model. "how do we teach how Past() works?"
<awygle>
i honestly believe that from a mental model "what will this do" standpoint what i'm describing will be more intuitive than the transformation-based approach
<awygle>
and (no disrespect) it may not feel that way to you because you're so much a language designer
<awygle>
not that you should take my word for it of course
<whitequark>
what happens when you use Past() to sample an output of a Verilog instance you're verifying with an nMigen specification?
<cr1901_modern>
awygle: FWIW, I think your original q of "why is it called Past() if it can be used with arbitrary clocks" is a reasonable q for ppl to ask when they play with formal.
<whitequark>
with #6, we can just punt to the user. they'll come up with some control sets for the outputs. if they're wrong? the behavior is identical. worst case, they turn the error into a warning and proceed
<whitequark>
with Past(), getting the annotation wrong can completely invalidate the spec
<awygle>
whitequark: if nMigen is able to be made aware of the control set of the Verilog output (e.g. the Verilog module also takes a clock as input), that information is used. if not, it should be an error
<whitequark>
actually, I trust you when you say that Past() intuitively means "past value of a signal" rather than "sampled value of an expression", and that it can well lead to a wrong mental model.
<whitequark>
I'm going to solve that by getting rid of Past() entirely and leaving only Sample() in the specification language.
<awygle>
i was thinking that might be the best solution tbh
<cr1901_modern>
>(10:49:40 PM) cr1901_modern: So $past simultaneously makes sense and doesn't make sense to me as a concept in all clock domains :P
<cr1901_modern>
Probably for the best wq
<whitequark>
I'm completely fine with it
<whitequark>
Assert(cnt == Sample(cnt) + 1)
<whitequark>
that... doesn't read all that well
<lsneff>
whitequark: If wasi had a serial or usb api, would you be interested in running programmers on wasm in addition to yosys/etc?
<whitequark>
lsneff: sure, if WASI does all the heavy lifting, i don't really have any reason to avoid that
<cr1901_modern>
>The use of $sampled in assertions, although allowed, is redundant, as the result of the function is identical to the sampled value of the expression itself used in the assertion.
<_whitenotifier-f>
[nmigen] whitequark commented on issue #534: Deprecate lexically dependent behavior of Past/Rose/Fell/... without explicit domain= argument - https://git.io/Jkv6X
<_whitenotifier-f>
[nmigen] whitequark closed issue #534: Deprecate lexically dependent behavior of Past/Rose/Fell/... without explicit domain= argument - https://git.io/JkvU3
PyroPeter_ has joined #nmigen
PyroPeter has quit [Ping timeout: 260 seconds]
PyroPeter_ is now known as PyroPeter
electronic_eel has quit [Ping timeout: 260 seconds]
electronic_eel has joined #nmigen
<_whitenotifier-f>
[nmigen] whitequark commented on issue #526: Behavior of Past/Rose/Fell/... is poorly suited for multiclock or asynchronous designs - https://git.io/Jkvix
<_whitenotifier-f>
[nmigen] whitequark edited a comment on issue #526: Behavior of Past/Rose/Fell/... is poorly suited for multiclock or asynchronous designs - https://git.io/Jkvix
<cr1901_modern>
whitequark: Just to be clear: Unless otherwise specified, nmigen formal is attempting to match the semantics of SVA?
<whitequark>
of the formalism underlying SVA, not the surface syntax
<whitequark>
(we can't really emulate the surface syntax even if we tried)
<whitequark>
er, sorry, you said "semantics". I need to sleep.
_whitelogger has joined #nmigen
emeb_mac has quit [Quit: Leaving.]
_whitelogger has joined #nmigen
<_whitenotifier-f>
[nmigen] anuejn commented on issue #526: Behavior of Past/Rose/Fell/... is poorly suited for multiclock or asynchronous designs - https://git.io/JkvbL
<_whitenotifier-f>
[nmigen] anuejn commented on issue #533: Breaking change in Record - https://git.io/JkvbC
lsneff has quit [Ping timeout: 260 seconds]
bubble_buster has quit [Ping timeout: 260 seconds]
Qyriad has quit [Ping timeout: 244 seconds]
sorear has quit [Ping timeout: 272 seconds]
alanvgreen has quit [Ping timeout: 272 seconds]
ktemkin has quit [Ping timeout: 272 seconds]
carlomaragno has quit [Ping timeout: 240 seconds]
daveshah has quit [Ping timeout: 246 seconds]
mithro has quit [Ping timeout: 260 seconds]
pdp7 has quit [Ping timeout: 260 seconds]
yuriks has quit [Ping timeout: 272 seconds]
esden has quit [Ping timeout: 240 seconds]
ianloic_ has quit [Ping timeout: 264 seconds]
tucanae47_ has quit [Ping timeout: 244 seconds]
guan has quit [Ping timeout: 260 seconds]
Cynthia has quit [Ping timeout: 272 seconds]
rohitksingh has quit [Ping timeout: 268 seconds]
key2 has quit [Ping timeout: 268 seconds]
tannewt has quit [Ping timeout: 264 seconds]
gravejac has quit [Ping timeout: 244 seconds]
_florent_ has quit [Ping timeout: 272 seconds]
guan has joined #nmigen
gravejac has joined #nmigen
ktemkin has joined #nmigen
ianloic_ has joined #nmigen
key2 has joined #nmigen
tucanae47_ has joined #nmigen
lsneff has joined #nmigen
sorear has joined #nmigen
yuriks has joined #nmigen
pdp7 has joined #nmigen
bubble_buster has joined #nmigen
daveshah has joined #nmigen
rohitksingh has joined #nmigen
esden has joined #nmigen
Qyriad has joined #nmigen
Cynthia has joined #nmigen
tannewt has joined #nmigen
mithro has joined #nmigen
_florent_ has joined #nmigen
carlomaragno has joined #nmigen
alanvgreen has joined #nmigen
<_whitenotifier-f>
[nmigen] anuejn commented on issue #525: [RFC] Add InstanceResource to use Instance I/O as Platform I/O - https://git.io/Jkvp9
<_whitenotifier-f>
[nmigen] anuejn edited a comment on issue #525: [RFC] Add InstanceResource to use Instance I/O as Platform I/O - https://git.io/Jkvp9
lsneff has quit [Ping timeout: 264 seconds]
lsneff has joined #nmigen
noknok has joined #nmigen
Asu has joined #nmigen
<_whitenotifier-f>
[nmigen] cestrauss opened issue #539: [RFC] Clarify the meaning of Past and friends when there is no lexically enclosing scope - https://git.io/Jkfm4
<cr1901_modern>
Okay, I'm going to come back to this later today, but mini minddump:
<cr1901_modern>
* The name "Sample" vs "Sampled" bothers me; seems easy to confuse if we have them both. I still prefer "Past", though I understand where awygle is coming from.
<cr1901_modern>
* "Past" in nmigen is an _alias_ of "Sample". I was reading an older SVA spec, but I think this is only true in certain cases. I will need to read the current spec and see if I'm misunderstanding something.
<cr1901_modern>
* In SVA, "$sampled" (sic), which seems to correspond to nmigen Sample (sic) is redundant in assertions. Why is this not the case in nmigen?
<cr1901_modern>
Okay, now I feel better for writing that down :D
<anuejn>
because that is essentially giving us a FWTF FIFO of depth 0 and there is no FWTF FIFO of depth 0?
<_whitenotifier-f>
[nmigen] cestrauss commented on issue #539: [RFC] Clarify the meaning of Past and friends when there is no lexically enclosing scope - https://git.io/JkfOM
_whitelogger has joined #nmigen
<d1b2>
<OmniTechnoMancer> Are resets in nMigen always synchronous?
<mwk>
not necessarily, you can create a ClockDomain with async_reset=True
<mwk>
but they are synchronous by default
<d1b2>
<OmniTechnoMancer> Ah okay, thanks
<cr1901_modern>
Does async_reset=True provide a synchronization circuit for releasing reset?
noknok has quit [Ping timeout: 246 seconds]
emeb has joined #nmigen
<d1b2>
<marble> I think I don't fully understand wishbone bus addresses in the context of bus width and granularity yet. If I attach a memory with 64 bit width and 8bit granularity, then the address would reference 64 bit wide segments, right? becuase the selection of each 8bit slice is done via the sel lines?
FFY00 has quit [Read error: Connection reset by peer]
<_whitenotifier-f>
[nmigen] RobertBaruch opened issue #540: Signal changes not on the clock during BMC - https://git.io/JkfMK
<_whitenotifier-f>
[nmigen] whitequark commented on issue #533: Breaking change in Record - https://git.io/JkfDY
<_whitenotifier-f>
[nmigen] whitequark edited a comment on issue #533: Breaking change in Record - https://git.io/JkfDY
<_whitenotifier-f>
[nmigen] whitequark edited a comment on issue #533: Breaking change in Record - https://git.io/JkfDY
korken89 has quit [Remote host closed the connection]
<_whitenotifier-f>
[nmigen] whitequark commented on issue #526: Behavior of Past/Rose/Fell/... is poorly suited for multiclock or asynchronous designs - https://git.io/JkfDj
<whitequark>
cr1901_modern: we wouldn't have *both* Sample and Sampled. only the latter one.
<whitequark>
cr1901_modern: Sample/Sampled in nMigen do not match anything in SVA so SVA is not a relevant comparison
<whitequark>
anuejn: depends on your point of view. my view is that any FIFO of depth 0 is always both empty and full, FWFT or not
FFY00 has joined #nmigen
<_whitenotifier-f>
[nmigen] whitequark commented on issue #540: Signal changes not on the clock during BMC - https://git.io/Jkf91
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Signal changes not on the clock during BMC - https://git.io/JkfQJ
emeb_mac has joined #nmigen
<_whitenotifier-f>
[nmigen] anuejn commented on issue #533: Breaking change in Record - https://git.io/Jkf5I
<_whitenotifier-f>
[nmigen] whitequark commented on issue #533: Breaking change in Record - https://git.io/Jkf58
Shari2 has joined #nmigen
<Shari2>
Hi. Anyone familiar with USB on the NeTV2? I can not find an UTMI or ULPI phy in the main board schematic. Does the NeTV2 support USB 2.0 High-speed?
ademski has quit [Remote host closed the connection]
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Signal changes not on the clock during BMC - https://git.io/JkfNm
ademski has joined #nmigen
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Signal changes not on the clock during BMC - https://git.io/JkfNB
<_whitenotifier-f>
[nmigen] RobertBaruch edited issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfMK
<_whitenotifier-f>
[nmigen] whitequark commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfNK
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfNQ
<_whitenotifier-f>
[nmigen] whitequark commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfAf
<_whitenotifier-f>
[nmigen] programmerjake commented on issue #443: add API to get simulation time for debugging - https://git.io/JkfAZ
<_whitenotifier-f>
[nmigen] whitequark commented on issue #443: add API to get simulation time for debugging - https://git.io/JkfA9
<_whitenotifier-f>
[nmigen] daveshah1 commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfA5
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfAN
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/Jkfxk
<_whitenotifier-f>
[nmigen] RobertBaruch edited a comment on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/Jkfxk
<_whitenotifier-f>
[nmigen] whitequark commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/Jkfxt
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfxO
<_whitenotifier-f>
[nmigen] whitequark commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfxZ
<_whitenotifier-f>
[nmigen] whitequark closed issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfMK
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #540: Another example of RTLIL failing BMC while Verilog succeeds. - https://git.io/JkfxV
<cr1901_modern>
>so SVA is not a relevant comparison
<cr1901_modern>
Then I think it's reasonable to document how Sampled() differs from SystemVerilog $sampled, since we are trying to match "the formalism underlying SVA". >>
<cr1901_modern>
I'm not sure how to do this other than I _think_ I found a way they differ (and am not prepared to defend my claim :P)
<whitequark>
Sampled() is SystemVerilog's $past
<whitequark>
you know, the more I think about it, the more I'm leaning towards ripping out the *entirety* of existing nmigen formal facilities and starting over
<cr1901_modern>
I don't have any particular qualms about that.
* cr1901_modern
says that because he has relatively little code to port over to any new system :P
<cr1901_modern>
whitequark: Anyways, my point was if Sampled == SVA $past, then someone who knows SVA but is trying nmigen may try to use Sampled as SVA $sampled
<cr1901_modern>
It wasn't me being pedantic for no reason
<whitequark>
satisfying you and awygle is mutually exclusive
<cr1901_modern>
Well, if that's the problem...
<cr1901_modern>
I'll get over it
<whitequark>
see, normally, it would be the case that I understand both language design *and* the domain area, which means I can stop bikeshedding by making a decision that's unlikely to be too bad. but formal specifications aren't my strong suit
<whitequark>
unfortunately you cannot design a coherent language by going through a string of individually reasonable decisions on contested issues, you have to think of how the parts will fit together from the very beginning
<_whitenotifier-f>
[nmigen] anuejn commented on issue #533: Breaking change in Record - https://git.io/JkfpR
<cr1901_modern>
I think RobertBaruch and I want the same thing- converting Verilog to nmigen naively, including the formal tasks that yosys support, should both produce the same answer when run in an SMT solver
<cr1901_modern>
brb
<cr1901_modern>
Anyways, like I said, whatever you decide, I'll "get over it", so don't let me stall you (_now_ brb)
<anuejn>
whitequark: ah okay I see that
<anuejn>
whitequark: would you think that it made sense if I wrote RFCs for Interface / PackedStruct now or is it better to wait because we are just before the 0.3 release and that would unnescessarily drain resources now?
<whitequark>
better to wait
<anuejn>
okay :)
<_whitenotifier-f>
[nmigen] whitequark commented on issue #533: Breaking change in Record - https://git.io/Jkfhn
<awygle>
Hrm. "Punt because needs comprehensive design" is on the one hand very sensible. On the other, bummer to lose what support we do have, esp since it's gonna be a long time before we can come up with that comprehensive design
<awygle>
Maybe I should try to troll a verification expert into joining the team :-P
<whitequark>
I think by this point the two primary choices I see are either that, or going ahead with my original plan for Past() and taking the hit from however many people will get confused by the semantics of that
<whitequark>
which one gets picked depends on how much delay are downstream users going to be fine with
<whitequark>
whatever decision is made will be done no earlier than 0.4, at least, that's certain
<whitequark>
the naming of Past() is possibly the single least important design question we discussed so far, and it has been completely unproductive
<awygle>
yeah, i agree with you
<whitequark>
I'm starting to see some of the same faults the Rust RFC process is being stalled hard by. I don't think they have a solution either, beyond "making a hard choice"
<awygle>
i almost want to slap DeprecationWarning on all of nmigen.asserts with a message like "hey this is prolly gonan change and we don't know how extensively, so uh, caution"
<awygle>
yeah, i said yesterday i was "starting to feel very Rust about this" and that's what i meant
<cr1901_modern>
I'm asking this in good faith: Are my points unreasonable?
<whitequark>
ahhh
<awygle>
no, not at all
<whitequark>
I think both cr1901_modern's and awygle's (and cesar[m]'s) points are reasonable
<awygle>
(imo)
<whitequark>
it's just that the effort being spent on this is neither justified by the magnitude of the problem, nor does it go towards solving it--in some ways perhaps the opposite
<awygle>
and all i really wanted from my bitching yesterday was that my concern was _understood_, not necessarily accepted/acted on
<awygle>
which i now feel satisfied with
<awygle>
whereever we go from here, i'm OK with
<whitequark>
that's good
<awygle>
and i also agree that i'd much rather be talking about "how does multiclock formal work" than "what is Past called"
<whitequark>
yep
FFY00 has quit [Remote host closed the connection]
<cr1901_modern>
whitequark: Did you see my privmsg re: multiclk?
<whitequark>
ironically, I don't think anyone expressed any opinions on multiclock formal
<whitequark>
cr1901_modern: yes. I really need to finish 0.3 though.
<awygle>
i have some but i got distracted lol
<whitequark>
ahh
FFY00 has joined #nmigen
<_whitenotifier-f>
[nmigen] anuejn commented on issue #533: Breaking change in Record - https://git.io/JkfjY
FFY00 has quit [Remote host closed the connection]
<awygle>
lol i alt-tabbed to firefox to close that tab and when i tabbed back there was a new comment posted by the bot
FFY00 has joined #nmigen
<_whitenotifier-f>
[nmigen] whitequark commented on issue #533: Breaking change in Record - https://git.io/JkfjZ
ademski has quit [Ping timeout: 245 seconds]
<whitequark>
hm. anyone up to fixing Record?
<anuejn>
I would just make it based on UserValue Again
<cr1901_modern>
whitequark: If you want my quick opinion on multiclk... it should unconditionally be "on" for nmigen, even with single domain designs. The convenience of multiclk off isn't worth all the shit that breaks when you forget to turn it on when you need it. Single domain _should_ be okay.
<awygle>
whitequark: i'll fix it, gimme 30m
<whitequark>
cr1901_modern: that's exactly my plan (which might have gotten lost a bit in that thread)
<anuejn>
Because what we are essentially doing is emulating UserValue using ValueCastable
<anuejn>
(imo)
<cr1901_modern>
ack
<awygle>
i agree w/ cr1901 but don't know how feasible that is in 0.3 timeframe
<cr1901_modern>
I admit I haven't fully read the thread after it... exploded
<whitequark>
anuejn: the reason it's migrated off Record is that UserValue will go in 0.4, but Record will remain for longer
<anuejn>
ah I see
<whitequark>
UserValue is fairly niche (it was made for LibreSOC but LibreSOC never used it), whereas Record is somewhat widely used
<whitequark>
awygle: I plan no formal changes whatsoever for 0.3 after all the discussion
<anuejn>
ah interesting
<awygle>
agree
<whitequark>
anuejn: the other reason is that making Record based on ValueCastable means people can extract and keep using it even once we remove it for good
<whitequark>
since ValueCastable isn't going anywhere
<anuejn>
it is one of the features i am emotionally really into because it allows you to create things where after writing them you can think "wow this was a nice hack" but I see why that not useful beyond making nice feelings ;)
<whitequark>
UserValue or Record?
<anuejn>
UserValue
<whitequark>
right
<whitequark>
that's actually sort of how it came into existence
<awygle>
hm for some reason i thought we'd end up lowering the ValueCastables automatically in the operator case but obviously that doesn't happen if the RHS is an int
<whitequark>
so I can relate
<whitequark>
awygle: yeah I think we should just replace the __getattr__ with a loop that proxies a predefined and fixed set of methods from Value
<whitequark>
ie generates wrappers and sets them on the Record class itself
<awygle>
whitequark: i'm gonna start with unit tests, because we should have some that catch this and also it'll help me know what exactly is broken
<whitequark>
yup
<whitequark>
sounds good
<whitequark>
i don't know what i prefer: this or generating elaborate contraptions that involve move constructors and move assignment operators in cxxrtl backend
<anuejn>
> i thought we'd end up lowering the ValueCastables automatically in the operator case
<anuejn>
is that the case?
<awygle>
case 1: record + signal - auto-lowering occurs
<awygle>
case 2: record + int - auto-lowering does not occur (because how could it)
<awygle>
(case 1 probably expands to all Values)
<anuejn>
(^^ I am not a lot into Record because I never fully understood it because of the confusing naming ;))
<awygle>
what we _really_ want is "in any operation involving a ValueCastable and a thing-that-can-be-turned-into-a-value, lower the ValueCastable"
<awygle>
("isn't a thing-that-can-be-turned-into-a-value a ValueCastable?" idk can you do "impl ValueCastable for int" in python? prolly not...)
<whitequark>
yes
<whitequark>
so, Value already implements this
<awygle>
yes
<whitequark>
hm
<awygle>
and it would be "questionable" to re-implement it just for record
<awygle>
hence the proxying
<whitequark>
wait, no, let's back off
<whitequark>
I see this a bit differently
<awygle>
ok
<whitequark>
what we want is "if you call a Value method (including dunder ones) on a Record, Record should be lowered to Value first"
<awygle>
yes
<awygle>
i can agree with that
<whitequark>
this means we can grab all Value methods we currently have, add a corresponding attribute to the Record class, and set it to a wrapper that casts self to Value
<whitequark>
by definition that is sufficient
<awygle>
hm. why doesn't our __getattr__ implementation do that
<whitequark>
because dunder methods don't go through __getattr__
<awygle>
ah -_-
<whitequark>
you can use __getattr__ on a metaclass, i think?
<whitequark>
but screw that
<whitequark>
it'll just create more problems down the line
<awygle>
yeah fuk dat
<awygle>
well i feel a bit better now
<awygle>
because rather than just "i didn't think of this" it was "my understanding of python minutae was insufficient"
<awygle>
in that case we don't need to wrap e.g. Value.bool, because that'll hit __getattr__, right?
<awygle>
(unless we want to drop __getattr__ entirely, which would be reasonable)
<whitequark>
technically yes, but if we *do* wrap that, we can get rid of the __getattr__ logic
<awygle>
yes
<awygle>
gotcha. ok, i'm on it
<whitequark>
thanks!
<awygle>
what's the rule on docstrings for "private" (_name) methods? are they still expected/desired?
<whitequark>
"if it makes things clearer"
<whitequark>
(when reading the source, since they don't go into the docs)
<awygle>
k
chipmuenk has quit [Quit: chipmuenk]
noknok has quit [Remote host closed the connection]
noknok has joined #nmigen
noknok has quit [Ping timeout: 260 seconds]
<awygle>
ugh python name-mangles __ methods that are _not_ builtins
<whitequark>
even those that end with __ ?
<awygle>
hm, no, apparently not
<awygle>
i specifically ran into this with __check_divisor and __check_shamt
<awygle>
those may not need proxying tho
<awygle>
dunno
<whitequark>
you shouldn't add proxies for _-methods
<whitequark>
other than operators
<awygle>
mk
<whitequark>
because if someone calls them on any Value it's their own fault
<awygle>
yeah i thought it was probable that they'd just work internally since we've already cast to value but i tossed them in my proxy loop just for safety. was gonna remove them and see if something broke lol
<whitequark>
basically yes, though I'd put _valueproxy outside of Record per se
<awygle>
k
<whitequark>
and then delete both _valueproxy and name so they don't pollute the global namespace
<whitequark>
yes, you can import `name` from `nmigen.hdl.rec` after the patch
<awygle>
wow
<whitequark>
and it'll have "eq" as the value
<whitequark>
in fact, you can even import "union" or "deprecated" or "tracer"
<whitequark>
python is kind of a crap language this way :/
<awygle>
just straight up using `del` feels gross, but anything else i can think of also feels gross, and probably wouldn't work anyway
<whitequark>
a while ago i thought that using __all__ would avoid that, but no, that only affects *
<awygle>
(e.g. wrapping it in "if True")
<whitequark>
yes, if you *don't* use __all__, then all that shit will end up downstream each time someone does import *
<whitequark>
which is why import * has a bad reputation
<whitequark>
well, the primary reason
<whitequark>
awygle: that won't work
<awygle>
right, didn't think so
<whitequark>
you can create a new scope with a function or a class
<awygle>
it'd have to go inside a fucntion and then you'd still have to delete that function
<whitequark>
and you can't create a new anonymous function
<awygle>
since it can't be a multiline lambda
<awygle>
ok fixed. prolly need a test for all those proxied functions, ideally....
<vup>
I have a question about 0 depth fifos. Currently those set `{w,r}_rdy` always to zero and do nothing else, what is the reasoning behind that?
<vup>
doesn't this just cause anything that uses them to stall?
<whitequark>
correct
<vup>
so could this not cause problems in generic code, that will silently fail if a fifo of depth zero is created for some reason?
<whitequark>
this is actually only useful for generic code, since nothing else would instantiate a zero depth FIFO
<vup>
ok, my question would be, in which case would this behaviour actually produce something that works?
<anuejn>
wouldnt it be reasonable (for fwtf fifos to connect w_en with r_rdy and w_rdy with r_en)?
<anuejn>
(and of course data)
<whitequark>
anuejn: no, because an FWFT FIFO still has one storage element
<whitequark>
I mean
<whitequark>
a FIFO where r_data changes when w_data changes regardless of whether w_en is asserted is clearly broken
<whitequark>
if a FIFO *ever* signals r_en, it must have storage
<whitequark>
*r_rdy
<whitequark>
vup: i remember the use case i had now
<anuejn>
whitequark: you are right
<whitequark>
... hm, no, i checked and that's not it
<whitequark>
vup: good question. i have no idea.
<vup>
so currently I can't think of myself either
<whitequark>
i can't figure out why i added this code
<vup>
*of one
<vup>
maybe nmigen could atleast emit a warning if one uses a depth=0?
<whitequark>
right now this is documented explicitly as what happens if you set depth=0
Asu has quit [Ping timeout: 256 seconds]
<whitequark>
this behavior is either desirable or not. if it is, there shouldn't be a warning. if it's not, it should be deprecated
<whitequark>
ah wait, i think i remember the motivation for it
<_whitenotifier-f>
[nmigen] RobertBaruch commented on issue #539: [RFC] Clarify the meaning of Past and friends when there is no lexically enclosing scope - https://git.io/JkJJ6
<_whitenotifier-f>
[nmigen] RobertBaruch edited a comment on issue #539: [RFC] Clarify the meaning of Past and friends when there is no lexically enclosing scope - https://git.io/JkJJ6
<_whitenotifier-f>
[nmigen] whitequark commented on issue #539: [RFC] Clarify the meaning of Past and friends when there is no lexically enclosing scope - https://git.io/JkJUU
<whitequark>
vup: iirc the logic was that since we have 0-wide signals and 0-deep memories, it makes sense to support 0-deep FIFOs
<vup>
hmm I am not sure the current behaviour (which is the only one that does not violate the FIFO contract) of 0-deep FIFOs is actually something that should be supported
<vup>
I don't think there is anything that would actually use a 0-deep FIFO if not for a bug
<vup>
whitequark: ^
<whitequark>
have you hit a problem with it?
<whitequark>
in glasgow, i have dummy FIFO interfaces that are inserted into slots of a crossbar that are not used
<whitequark>
which is similar although it does not actually instantiate such FIFOs
<whitequark>
ultimately, it seems to be similar to the argument against 0-bit signals
<anuejn>
I had a problem with it althogh i have to admit that the current behaviour is clearly stated in the documentation
<whitequark>
what was the problem?
<anuejn>
I am wrapping the fifos in a stream fifo and was expecting that that fifo of depth 0 would just pass the input to the output without me special casing it in my stream fifo implementation
<whitequark>
right
<anuejn>
but i see that this is a thing that would be reasonable to do (in my stream fifo)
<whitequark>
i think this is not an argument for e.g. showing a warning as vup suggests
<anuejn>
agreed
<vup>
also I think with 0-bit signals (due to generic code) you can actually get something that still does something / what you want it to do. With a 0-depth fifo it will just stall
<whitequark>
well, yes; that is exactly what i want in the fx2 crossbar
<whitequark>
the idea is that if you want to "disable" a FIFO while leaving the interface around, you can use zero depth
<anuejn>
sounds reasonable
<whitequark>
zero-deep FIFOs are a lot less useful than zero-width Signals because FIFOs are in general a lot less useful than Signals
<whitequark>
(in the sense of use frequency)
<whitequark>
if there was any ambiguity over what it should do, I would have agreed, but there is only one thing it can do that satisfies the contract
<vup>
I understand, it just feels like something I will hit sooner or later due to a off-by-one error und will then spend a lot of time searching for.
<vup>
I guess this is where lints / custom lints would come in.
<whitequark>
hm
<whitequark>
it's not something I would lint against
<whitequark>
(just like 0-wide signals)
<vup>
yeah I am saying I would want to lint *my* code against that.
<whitequark>
right
<whitequark>
ok, yeah, i can see why you'd use a custom lint