peepsalot has quit [Read error: Connection reset by peer]
emeb_mac has quit [Ping timeout: 252 seconds]
bvernoux has joined #nmigen
miek has quit [Ping timeout: 250 seconds]
pftbest has quit [Remote host closed the connection]
pftbest has joined #nmigen
pftbest has quit [Ping timeout: 260 seconds]
peepsalot has joined #nmigen
<_whitenotifier-1>
[nmigen] doronbehar opened pull request #638: Mention a bit the origin of migen and m-labs - https://git.io/J2OSK
<_whitenotifier-1>
[nmigen] whitequark commented on pull request #638: Mention a bit the origin of migen and m-labs - https://git.io/J2O7T
<_whitenotifier-1>
[nmigen] whitequark closed pull request #638: Mention a bit the origin of migen and m-labs - https://git.io/J2OSK
<_whitenotifier-1>
[nmigen] whitequark commented on pull request #638: Mention a bit the origin of migen and m-labs - https://git.io/J2O5h
<d1b2>
<widlarizer> Is there a way to use Records with decoders? I would like to group my RV32I processor's control signals and still have them decode in gtkwave without having to assign them to module-local signals
pftbest has joined #nmigen
<vup>
@widlarizer: you can do that somewhat
<vup>
you can specify your own Signal's for (some) of the record fields using the `fields` argument of `Record`
<whitequark>
it's an odd edge case because calling Repl with an int as a first argument is probably a bad idea
<whitequark>
I feel like this should be a warning or something
<lkcl>
i agree - i mean, i _did_ it, and Nothing Bad Happened :)
<lkcl>
"Repl(0, len(x)" was the actual code
<lkcl>
where it should, clearly, have been "Repl(Const(0, 1), len(x))"
<whitequark>
Repl(0, len(x)) is just Const(0, len(x))...
<lkcl>
it was code i wrote when first starting out :)
<lkcl>
doh
<whitequark>
the same considerations apply to Cat, too
* lkcl
thinks...
<lkcl>
the potential implications about such a warning, though... i think they kinda ripple out throughout the whole of nmigen
<lkcl>
where integers are used.
<whitequark>
where bare integers are used in a context where the width is important
<lkcl>
which... would imply... thinking it through... that it would be Const itself that would need the warning about not having a length argument
<lkcl>
ahh yeah
<lkcl>
where some (which i honestly don't know about) wouldn't matter, ok got it
<whitequark>
the underlying reason is the conflation of the two uses of Python `int` in nMigen: as integral numbers (which is how arithmetics works) and as bit sequences (which is how slicing, concatenation, replication works)
FL4SHK has joined #nmigen
<FL4SHK>
hello, I return!
<lkcl>
apologies, i am frickin freezing. temperature's dropped fast here in the UK and i haven't quite adjusted yet. must get some food.
<whitequark>
this conflation happening because Value is used for both purposes, and integers are value-castable
<lkcl>
whitequark, will be back. i'm shivering. have to warm up
<FL4SHK>
I bought a ZCU104 evaluation board.
<FL4SHK>
Seems you can't use the yosys flow with UltraScale+ yet.
<whitequark>
I thought that Yosys Xilinx flows are fairly immature in general
<FL4SHK>
Isn't it tied to 7 series only?
<FL4SHK>
Oh, they're immature.
<whitequark>
(in any case, there's a reason nMigen supports ISE and Vivado as first-class toolchains)
<FL4SHK>
Can't forget Quartus.
<FL4SHK>
I just installed Vivado.
<_whitenotifier-1>
[nmigen] whitequark opened issue #639: Consider issuing a warning when a bare integer is used in Cat() or Repl() - https://git.io/J23xS
<FL4SHK>
`Record` is still going to be replaced, right?
<whitequark>
yes
<_whitenotifier-1>
[nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1] https://git.io/J23pI
<_whitenotifier-1>
[nmigen/nmigen] whitequark 65499d5 - hdl.ast: add tests for casting bare integers in {Cat,Repl}.
<FL4SHK>
Okay so is there any plan for something like storing a packed array of structs?
<whitequark>
maybe
<_whitenotifier-1>
[nmigen/nmigen] github-actions[bot] pushed 1 commit to gh-pages [+0/-0/±13] https://git.io/J23pc
<_whitenotifier-1>
[nmigen/nmigen] whitequark 56dbcb8 - Deploying to gh-pages from @ 65499d5c45a2025b198ba2be9f3c694b028e3d30 🚀
<whitequark>
would need to write an RFC and discuss it; that's still in the future
<FL4SHK>
Well, what does writing an RFC consist of?
<whitequark>
an RFC is a proposal for a design that can be discussed and evaluated by all interested parties
<FL4SHK>
I see, I see.
<whitequark>
Records specifically are tricky
<whitequark>
I'll likely write an RFC for that myself in a few weeks, and then we'll see where that leads us
<FL4SHK>
I want things like records containing arrays of records, being as general as possible.
<whitequark>
I very much agree that the current design is unsatisfactory for many reasons
<FL4SHK>
All right. I'm willing to contribute to this.
<whitequark>
let's discuss this once there's bandwidth to do it, then
<FL4SHK>
Cool. I really appreciate all you guys' hard work on nMigen. It's my favorite HDL.
<whitequark>
thank you 🧡
<FL4SHK>
That's why I'm a Patron.
<FL4SHK>
Did I say something strangely?
<whitequark>
nope
<FL4SHK>
Cool.
<whitequark>
i'm just glad you like it
<FL4SHK>
I've got a lot of work ahead of me for my big HDL project.
<FL4SHK>
I'm attempting to construct an FPGA-based desktop.
<whitequark>
neat!
<FL4SHK>
That's why I bought the ZCU104 evaluation board.
<FL4SHK>
It allows me more performance (and especially RAM) than something like an RPi.
<FL4SHK>
Original RPi, anyway.
<FL4SHK>
I'll be having to run a source-based OS.
<FL4SHK>
...will be keeping around an x86-64 laptop
<FL4SHK>
I don't think I can get away completely from either ARM or x86-64.
<FL4SHK>
I recently learned that you can do 4k 60 video signal generation with Ultrascale+ FPGAs.
<FL4SHK>
This is a huge deal, as 4K I think is the largest resolution I think I'll ever be using.
<FL4SHK>
SERDES is key.
* lkcl
waves to FL4SHK
<lkcl>
woo that's a beefy bit of kit. was it... lethalbit who was doing xilinx ultrascale fpga support for nmigen?
* lkcl
had a brainwave / idea
<lkcl>
has it ever been discussed or considered to create an ast.Bool?
<lkcl>
the reason is that casting in Type 2 (dsl.Module) down to Type 1 (ast.*) of "boolean-looking" values is an explicit detection
bvernoux has quit [Quit: Leaving]
<lkcl>
i.e. is the last-remaining inter-linkage between Type 2 (dsl) and Type 1 (ast) that is non-abstracted
<lkcl>
using "if len(val) != 1" to do so
<lkcl>
there are four areas (identified so far) in nmigen where that test is explicitly made:
<lkcl>
i initially considered the idea of adding a Value.is_considered_bool() function
<lkcl>
but that is just as nebulous / non-abstracted and still crosses the boundary between Type 2 (dsl) high-level nmigen language constructs, and Type 1 (ast) low-level nmigen language constructs
<whitequark>
nmigen operates on bit vectors (or values castable to them)
<lkcl>
indeed.
<whitequark>
.bool() isn't any more fundamental than .all() or .any()
<lkcl>
python not having overrides on "and" and "or" is one of the pain-in-the-heck things
<whitequark>
it's a way to reduce a bit vector to a single-bit value
<lkcl>
ah i don't mean bool() the operator
<lkcl>
i mean, "ast.Bool" as a higher-level concept
<lkcl>
such that rather than this:
<lkcl>
if (len(val) != 1) val = Value.cast(val).bool()
<lkcl>
those two lines would just reduce to:
<lkcl>
val = Bool.cast(val)
<DX-MON>
lkcl: lethlabit and us were.. are.. doing Zynq PS{7,8} support, yes
<lkcl>
DX-MON: niiiice. i'm really looking forward to that because i have a ZC706 i've been completely unable to use for 3 years
<whitequark>
of the four instances you identified, two are in nmigen.compat and are irrelevant to nmigen's design
DX-MON is now known as dragonmux
<whitequark>
the other two are an implementation detail
<whitequark>
semantically, `Value.cast(val).bool()` would be just as fine
<whitequark>
the only reason the length test is there is because it makes the Verilog a little bit more readable
<whitequark>
I actually think we might not need the cast in `Mux()` at all
<lkcl>
it seems so trivial a thing, so small, yet is (as best i can tell) literally the last remaining blocker to full abstraction between Type 1 (ast) and Type 2 (dsl)
<whitequark>
I don't see anything that it would block
<whitequark>
and I don't see an addition of a new toplevel name to the prelude as a small or trivial thing
<lkcl>
yeah which is why i'm hesitant to raise it
<lkcl>
there's one unit test for Mux which passes in an actual boolean true/false as the "sel" argument
<whitequark>
for Mux, Value.cast should be enough, I think?
<lkcl>
the current code is doing a Value.cast() followed by detection of the length, and an explicit bool() operation added if the length is greater than one.
<lkcl>
which makes sense: it would be a waste to add the bool() operation on something that's already going to be a one bit test
<lkcl>
the "abstraction" i am referring to is to move those two lines (the length-test and the bool-operator-call) into a (new, proposed) ast.Bool
<lkcl>
dsl.Module could then use that
<lkcl>
(that code - the 2 lines - the "nebulous heuristic detection" - are duplicated inside of dsl.Module)
<_whitenotifier-1>
[nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±4] https://git.io/J2sO2
<_whitenotifier-1>
[nmigen/nmigen] github-actions[bot] pushed 1 commit to gh-pages [+0/-0/±13] https://git.io/J2sO5
<_whitenotifier-1>
[nmigen/nmigen] whitequark 37cd33e - Deploying to gh-pages from @ e88d283ed30448ed5fe3ba264e3e56b48f2a4982 🚀
<lkcl>
err... if i can work out how from the links :) oh got it. git.io.
<_whitenotifier-1>
[nmigen] adamgreig commented on issue #639: Consider issuing a warning when a bare integer is used in Cat() or Repl() - https://git.io/J2s3r
<lkcl>
okaaay, right, so... it's move into the actual rtlil. that makes a lot of sense
<_whitenotifier-1>
[nmigen] whitequark commented on issue #639: Consider issuing a warning when a bare integer is used in Cat() or Repl() - https://git.io/J2ssY
<whitequark>
what about it?
<lkcl>
with the Mux one gone (which is fantastic), that's literally the last-remaining (2 lines of code) that is an explicit (heuristical) link between Type 2 (dsl) nmigen language constructs
<lkcl>
and Type 1 (ast) lower-level nmigen language constructs
<whitequark>
I don't really see the distinction you're making
<whitequark>
those two lines are an implementation detail of the `If`/`Elif`/`Else` construct
<lkcl>
dsl.Module (type 2 nmigen language high-level constructs) is entirely abstracted to be in terms of ast.* (type 1 nmigen low-level language constructs)
<lkcl>
... except for that *one* place.
<lkcl>
comprising two lines of code
<lkcl>
which is quite an amazing achievement
<lkcl>
one of the reasons is because dsl.Module entirely thunks down to ast.Switch. i.e. is implemented in terms of ast.Switch. m.FSM, m.If/Else/Elif, m.Switch, they all thunk onto ast.Switch
<whitequark>
yes
<whitequark>
that's the only reason, actually
<lkcl>
but those two lines are *the* only location in the entirety of dsl.Module where that abstraction is broken
<lkcl>
if those two lines were removed, to be replaced with just "tests.append(if_test)"
<lkcl>
and it became *ast.Switch's* responsibility to perform that test
<whitequark>
a different design would have a separate AST construct for `If`/`Elif`/`Else`
<lkcl>
*now* dsl.Module would be 100% fully abstracted
<whitequark>
such a design would be able to produce slightly more human-readable code in the backend, too
<lkcl>
oh, sorry, and the Value.cast
<whitequark>
ultimately, I don't see `nmigen.ast` and `nmigen.dsl` as having an intentional abstraction boundary between them
<lkcl>
3 lines. 436 to 438
<lkcl>
i do appreciate that: which makes the fact that they _are_ abstracted all the more impressive :)
<whitequark>
they aren't
<whitequark>
they just look a bit like that
<whitequark>
an abstraction boundary is a function of -intent-; it doesn't live in the code but in the people who maintain it
<whitequark>
the code reflects the intent, not the other way around
costledger_ has joined #nmigen
costledger_ has left #nmigen [#nmigen]
<lkcl>
i see where you're coming from. i'm hestitating and reluctant to explain further. i am glad though to have helped highlight the Repl thing, and relieved that one of the blockers (Mux) is moved to the correct place (rtlil.py)
<lkcl>
agg: yes i've done the same thing in a number of places. https://git.io/J2s3r
<agg>
I've also seen Const(0) and Const(1) used in that situation
<agg>
Const specifies it defaults the width to the minimum required for the integer if unspecified, so it's better defined in that case, but a bit more long-winded to type out
<agg>
also my brain sees C(0, 1) and goes "oh, is it a 1-width 0 or a 0-width 1???? oh.. duh"
<dragonmux>
we'd point out agg that you can say shape=1 for that second parameter if that helps you disambiguate
<agg>
that would help! though does make it even more long-winded compared to using a bare 0/1 in Cat, at least
<agg>
if you permit bare 0/1 in Cat though I guess you can't tell if the user wrote `0` or passed in some variable with a 0 value that might not be 0 the next time, hm
<agg>
(well of course it's possible to tell with enough metaprogramming)
<lkcl>
whitequark, can i possibly check something with you? i believe the call to if_test=Value.cast(if_test) may be redundant due to _check_signed_cond() always being used, and always doing a Value.cast(cond)
<lkcl>
unless i've missed something, those are the only two cases and they both already perform the Value.cast() ?
<lkcl>
i leave it with you - it's Saturday :0
<lkcl>
:)
kmehall_ is now known as kmehall
<FL4SHK>
How do I use `ValueCastable`?
<FL4SHK>
First time coming back to this code in months, maybe a year.
emeb_mac has joined #nmigen
<vup>
FL4SHK: well you inherit from `ValueCastable` and then annotate a method of your choice with @ValueCastable.lowermethod. This method should return some kind of `Value` (`Signal` or `Const` or some expression made up of those)
<FL4SHK>
I see.
<FL4SHK>
That seems to be what I did with my existing code.
<FL4SHK>
Can I cast some of the bits of a signal to a `Record`?
<vup>
you can `m.d.comb += record.eq(signal[some_slice])`, but I assume that is not what you want?
<FL4SHK>
I'm not sure if that's what I want.
<FL4SHK>
It might work, though.
<FL4SHK>
I've got some code written for a packed array.
<FL4SHK>
I'd like to allow elements to be `Record`s or at least to have something like a `Layout`
<FL4SHK>
It'd be nice to do `my_packarr[3].a`
<FL4SHK>
I suppose just doing `m.d.comb += record.eq(...)` might be enough.
pftbest has quit [Remote host closed the connection]
<vup>
sure that seems like it should work. It might even be easier to just write a alternative to `Record` that mimics its interface and supports "casting"
<FL4SHK>
That'd work for me.
<vup>
(that would atleast generate a bit cleaner rtlil / verilog I think)
<FL4SHK>
How would I go about doing the first option?
<FL4SHK>
I believe it'd need to involve nMigen shapes, but I don't recall what to do.
pftbest has joined #nmigen
pftbest has quit [Remote host closed the connection]
<FL4SHK>
Actually, I'd like to do the latter.
<FL4SHK>
Looking at `Record`, I'm getting ideas about how this is supposed to be done.
pftbest has joined #nmigen
pftbest has quit [Ping timeout: 246 seconds]
<vup>
As a start you can probably copy `Record` and then instead of building it out of fields of `Record`s and `Signal`s back it by a single `Signal`
<whitequark>
lkcl: yes, it's redundant
<_whitenotifier-1>
[nmigen/nmigen] whitequark pushed 1 commit to master [+0/-0/±1] https://git.io/J2C9R
<FL4SHK>
vup: So what's the deal with building it with only a single backing signal?
<FL4SHK>
I'd like to be able to do `a.b[3].c`
<_whitenotifier-1>
[nmigen/nmigen] github-actions[bot] pushed 1 commit to gh-pages [+0/-0/±13] https://git.io/J2CQZ
<_whitenotifier-1>
[nmigen/nmigen] whitequark 623b8bb - Deploying to gh-pages from @ fac1b4b2d1b6db15339ffc601c72b655b6df6e77 🚀
<vup>
FL4SHK: well building it with only a single backing signal does not preclude you from doing that?
<vup>
a.b[3].c mainly needs a way to specify arrays as part of a `Layout`
<FL4SHK>
I see.
<vup>
but using a single backing signal (or rather `Value`) would make it easy to "cast" a `Value` to be of that Record-like thing
<vup>
because you can then just swap out the backing `Value` / signal
<FL4SHK>
Can't you just have a flattening method that you call?
<FL4SHK>
That's what I had done previously.
<FL4SHK>
I implemented something like this before.
<vup>
a flattening method you call on what?
<FL4SHK>
Is the goal to only have a single signal at the end?
<FL4SHK>
I call it on the packrec.
<vup>
I thought you goal was having a packed array of records and you needed some way to transforming a (internal) element of that array (which would just be a part of a `Signal`) into something that is (or atleast looks like) a `Record`
<FL4SHK>
I believe that can be done by returning a `Packarr` from `a.b` if `a.b` is actually a packed array.
<vup>
it sounds to me like you are talking about the other direction from what I described earlier?
<vup>
(putting a packed array into a records vs putting records into a array)
<FL4SHK>
I'd need to support both putting a packed array into a record and putting a record into a packed array
<vup>
sure
<vup>
but as far as I understand a packed array is currently just a `Signal`, right?
<FL4SHK>
I'd need to change that.
<FL4SHK>
How would you go about casting the output of `my_signal.word_select()`
<FL4SHK>
That's something I need.
<vup>
Sure I thought that was what we were talking about the whole time :)
<FL4SHK>
HMMMM
<FL4SHK>
Yes, you're right.
<FL4SHK>
I suppose I just confused.
<FL4SHK>
The thing is
<FL4SHK>
If I use a backing `Signal` in a `Packarr`, I still need to store the shape of the elemetns.
<FL4SHK>
And so in the `Packrec` class I'd need to keep track of what type each element is as well.
<FL4SHK>
Using a backing signal in only `Packarr` is what I'd like to do
<vup>
what do you mean by keeping track of what type each element is is `Packrec`?
<vup>
Do you not always need to do this?
<FL4SHK>
You're right.
<FL4SHK>
I'd like for `Packarr` to take, as a constructor argument, the nMigen `Shape` of each element
<FL4SHK>
And thus `Packarr` needs to have a `Shape` itself.
<vup>
wait of each element?
<FL4SHK>
Same type per element
<FL4SHK>
*same shape
<vup>
Oh ok
<FL4SHK>
Can you do a `word_select()` of a `word_select()`?