<adamgreig[m]>
it's maybe best to think of the sequential/registered signals as a part of your state rather than strictly as outputs I guess. have you tried the ongoing() method? it's quite useful for setting combinatorial outputs that need to be set based on just what state(s) you're in/not in
<jjsuperpower>
I don't fully understand your first question. My understanding is typically in HDL languages if you may set a combinational signal equal to some other signal or constant, conditionally. This is implemented using a latch. I would like the combinational signal to not change in state 2, behaving like a latched signal.
<adamgreig[m]>
ah, so it's the behaviour for combinatorial signals you have an issue with, not the behaviour for sequential/registered signals?
<adamgreig[m]>
are you targetting an fpga or an asic? I don't have experience with asics, but my understanding is that you don't really want to infer latches on fpgas
<jjsuperpower>
fpga
<adamgreig[m]>
so in that case, it might be better to use a register for that signal, or otherwise to recompute the output signal as required... is it really a mealy state machine if the output signal depends on its past value as well as the current state and inputs?
<whitequark>
for context, the issue with FPGAs and latches is that timing analysis doesn't handle those well
<jjsuperpower>
yes, i know
<jjsuperpower>
Correctly, if I am wrong, but a mealy machine output is dependent on the current inputs and the current state
<jjsuperpower>
so what if I to change which input signal influences the output based on a given state?
<jjsuperpower>
From what I learning in Undergrad (I'm a grad student currently) mealy outputs can be asynchronous. Not saying this is ideal btw.
<adamgreig[m]>
it sounds like your `output` currently depends on the state and its own current value, which wouldn't normally be an "input"
<adamgreig[m]>
for fpga work you will probably have a better time if you can keep things synchronous - in this case perhaps you can depend on outputs previous value (in other words, make it sequential logic) and then amaranth will also do what you want
<adamgreig[m]>
or, perhaps what you want is for `output` to be a combinatorial function of the inputs, but the function is different for different states? so maybe you have inputs A and B, and in state1 you want output to be `A & B` but in state2 it's `A | B` combinatorially... that would be OK too, and also worth OK in amaranth, but doesn't involve latches either
<jjsuperpower>
I mean yes in general that is the rule of thumb. Let me give you more context on what I am working on
<jjsuperpower>
I am working on creating a IP with an basic axi-stream input and output. The idea is to do some mathmatical operations on data from a dataflow approach.
<jjsuperpower>
This of course involves pipelining. There are times when the IP axis-slave ready (IP output) signal should be dependent on the IP axis-master (IP input), but other times the axis-slave should be inactive due computations
<jjsuperpower>
* IP axis-master ready signal
<jjsuperpower>
Long story short I am trying to use a mealy machine to control the dataflow.
lf has quit [Ping timeout: 248 seconds]
lf has joined #amaranth-lang
<adamgreig[m]>
I don't fully see how that relates to the example in your first question with state1/state2/output, but can you not use a register for the ready output? having it go through a long combinatorial path in your module probably won't be good for timing anyway
<jjsuperpower>
For the record I am new to doing stuff with axis stream. I think you are correct in that regard. I need to check if a synchronous ready signal would cause and issue in a full pipeline
<_whitenotifier-9>
[YoWASP/yosys] whitequark ea0fe06 - Factor out runtime code into yowasp-runtime.
<jjsuperpower>
adamgreig[m] You had mentioned that Amaranth does not support latches. Are you sure you are correct? I did read the documentation you sent me. The reason I am questioning your answer is for example, isn't an enable signal on a counter implemented using a syncronous latch?
<adamgreig[m]>
the link (https://amaranth-lang.org/docs/amaranth/latest/lang.html#combinatorial-evaluation) is more precise: amaranth doesn't allow using combinatorial feedback loops to implement transparent latches (and won't infer them). it's possible to have a latch by directly instantiating the relevant hardware blocks
<_whitenotifier-9>
[amaranth-lang/amaranth-yosys] whitequark cd96383 - [autorelease] Update wasmtime version requirement from <2 to <6.
<jjsuperpower>
Yes, that makes sense. Sorry if I misquoted you
<adamgreig[m]>
so it's transparent/asynchronous latches specifically that aren't permitted, though for the counter case I'm not sure I'd consider it a synchronous latch for a counter with enable? it's not sensitive to edges in the enable signal, it considers the enable signal on each clock edge instead
<jjsuperpower>
I fully agree, I should have been more clear on what I meant by synchronous latch.
<jjsuperpower>
But back to my original thought. If I manually create FSM without the Amaranth FSM class, I would expect the combinational logic to not change in a state where it is not modified (I have not tested this in Amaranth, I know this works in VHDL)
<jjsuperpower>
However, when I use the FSM class provided by Amaranth I do not get the expected behavior. That is why I asked about it here. Btw thank you for your time, I really appreciate it.
<adamgreig[m]>
no problem! that's just how combinatorial signals work all across Amaranth: the guide says "The final value of a combinatorial signal is equal to its initial value updated by the active assignments in the assignment order.", and the assignments from another state are not active
<adamgreig[m]>
so a combinatorial signal with no active assignments will take its reset value, rather than inferring a latch to an old value
<jjsuperpower>
Well, I am glad to know its not a bug. Its a little annoying, but I am not complaining. I think Amaranth is really cool (and useful) tool. Besides, every tool has its quirks.
<adamgreig[m]>
arguably, the quick is in VHDL for inferring a latch!
<whitequark>
there's been a few complicated solutions proposed in the issue and I think the author is no longer active in the community, so I'm going to ask if you folks see any problems with mine or is that good to go
Guest40 has joined #amaranth-lang
<cr1901>
It doesn't look much different from what's there now (no objections)
<_whitenotifier-9>
[amaranth-lang/amaranth-lang.github.io] whitequark 69a45da - Deploying to main from @ amaranth-lang/amaranth@666ee27fd0fa9fb90109e0e201f878f9ab034c85 🚀
<whitequark>
currently, in build system overrides, you can pass e.g. nextpnr_opts=['foo', 'bar'] and it's the same as nextpnr_opts='foo bar'. considering that overrides are explicitly designed to be used from both Python source and command line, I feel like we're better off restricting that to strings specifically
<whitequark>
since then the parity between the interfaces is obvious
<whitequark>
otherwise we end up with a tiny faux-type-system where various code passing overrides around uses sometimes strings and sometimes lists
<whitequark>
the other issue is that the way you can currently pass them as arrays you would think that they're properly quoted for the shell. but they are not at all
<whitequark>
if we ever decide to pass around structured overrides I think they should be serialized into something well-defined like json first so you can do it on the command line as well as through Python
<whitequark>
but the current interface is built around dumb textual substitution and I think exposing that to the user is ok
<adamgreig[m]>
(I'm currently "operating heavy machinery" so can't chime in, sorry!)
<whitequark>
will leave (2) until adamgreig can chime in later
<whitequark>
to remind everyone, the syntax for with m.Case(...) is that ... can be one argument (everyone's familiar with this), two arguments (which become an or-pattern), or zero arguments (which... well, it's always an or pattern, and an or of zero patterns is always true). so with m.Case(): is the default case essentially
<tpw_rules>
re (2), can/should we not fix them to be properly quoted?
<whitequark>
tpw_rules: how do you pass multiple arguments on the command line then?
<whitequark>
I guess a better way to ask this question is: how do you achieve parity between command line overrides and kwargs-based overrides?
<tpw_rules>
you could use multiple instances of the option? maybe name it in a singular tense. idk how often nextpnr or other tools use consecutive arguments (i.e. '-x v' vs '-x=v')
<whitequark>
the override when used on command line isn't an option, it's an environment var AMARANTH_nextpnr_opts
<tpw_rules>
i see. i'm not sure then, we can move on
<whitequark>
the other thing is that right now command_templates is a polyglot
<whitequark>
the same templates are used for sh, bash, and cmd
<whitequark>
emit_commands() takes a syntax argument, so I guess that could be threaded into the options filter in the rendered template
<whitequark>
but at some point we have to ask ourselves, how much % of Amaranth do we want to turn into a complex shell templating engine
<whitequark>
also, overrides are typically used for "I want to specify a seed real quick" or "I want to disable timing" or "my weird board requires this weird bitgen option" which aren't exactly quoting heavy workloads
<whitequark>
re (3), every case past the default case will be ignored
<jfng[m]>
do we often use with m.Case(), instead of m.Default() ?
<jfng[m]>
i.e. that could be an indicator of a typo
<whitequark>
it's possible to end up with a super-exhaustive case statement in other way, e.g. with m.Case(1): followed by with m.Case(1): will have the second one be ignored. but with m.Case(): doesn't require any cross-case analysis
<whitequark>
and also exhaustiveness checks in general are computationally hard, I think maybe NP-complete?
<whitequark>
Yosys is not doing a good job there either
<whitequark>
jfng: I think a literal `m.Case()` may not be very useful yeah, other than in generated code
<whitequark>
it's taking *args so it's difficult to detect the case of m.Case() specifically
<whitequark>
to make a warning there
<whitequark>
maybe look up the source line and match it for .Case()? that's cursed as hell but it would in practice have essentially no false hits
<jfng[m]>
i like the idea of the warning in #435, but if it was an unintended m.Case() typo, it would be slightly misleading
<whitequark>
so we have two options here
<Chips4MakersakaS>
Isn't len(args) == 0 for m.Case() ?
<whitequark>
3.1) always warn on literal m.Case() and avoid the problem by virtue of m.Default() being explicit about intent (but see below)
<whitequark>
3.2) warn when any m.Case() or m.Default() is followed by m.Case(...)
<whitequark>
Chips4Makers (aka Staf Verhaegen): you might want to emit a switch in generated code with a loop, where you'd have something like `for cases, body: with m.Case(*cases): body()`
<whitequark>
and then you have m.Case(*[]) which is not the same as a literal m.Case()
<whitequark>
well not generated code, in code that uses metaprogramming
<tpw_rules>
i've definitely done that.
<whitequark>
sure you can do a branch like if cases: with m.Case(): else: with m.Default(): but that goes contrary to our tradition of enabling metaprogramming, not making it more annoying
<whitequark>
re "see below": there are three mostly orthogonal issues at play here. (a) exhaustiveness checking; (b) avoiding misleading looking / produced by a typo `m.Case():`; and (c) new Amaranth designers not realizing `m.Switch` is a priority case statement
<whitequark>
(b) is relevant regardless, I think having that in user code will be at the very least confusing since it's a super uncommon statement
<whitequark>
(c) is helped by (a) in that if you don't understand it's a priority case an exhaustiveness check will quickly alert you to that fact (or just confuse further)
<whitequark>
but implementing (a) is a very heavyweight task
<whitequark>
solution (3.2) is a very minimal form of exhaustiveness checking that we can definitely do, but I'm kind of meh on shipping that since people will want other "obvious" things like `Case(1)` ... `Case(1)` and then eventually patterns
<jfng[m]>
yes, that's what i was thinking of also
<jfng[m]>
this m.Default() overlap is just a specific case
<jfng[m]>
with e.g. range patterns
<whitequark>
and once you detect overlapping case items you have to have a linter override
<whitequark>
that we don't have a design for
<whitequark>
because those are really common in metaprogrammed code
<jfng[m]>
could this check be done in yosys, maybe ?
<whitequark>
which part would that solve?
<jfng[m]>
detecting unused cases, as a result of overlaps
<jfng[m]>
a bit like proc_prune, but for switch cases ?
<whitequark>
sure, yosys has proc_rmdead (which can get exponential with some patterns like 1 1- 1-- 1--- ... 1-(64 bits)-)
<whitequark>
but how do you communicate back which pattern corresponded to which Amaranth source line?
<whitequark>
in Yosys case patterns do have attributes at least and we emit \src for those, but Yosys doesn't have machine-readable diagnostics
<whitequark>
besides, we still have the linter override question
<whitequark>
I agree that unique/exhaustive case is something that's going to be desired by at least some people
<whitequark>
so we should have it eventually, via Yosys or not
<whitequark>
but, should we gate the entire feature on that?
<whitequark>
i.e. whichever feature detects unintentional use of m.Case()
<jfng[m]>
FFY00 did 3.2) in a few lines of code
<whitequark>
that doesn't handle the case where you intentionally have a case after the default in metaprogrammed code
<whitequark>
it also doesn't handle the case where you make a typo in m.Case(): but it's the last one in the switch
<whitequark>
*make a typo that results in m.Case() rather than m.Case(0) or something
<jfng[m]>
ah, yeah right
<jfng[m]>
my point was, I agree that a proper exhaustive check can wait, if we can address the common cases easily
<whitequark>
the thing with linter checks is that so long we can't turn them off (and we can't now), they should have a 0% false positive rate
<whitequark>
the only way I can see this being handled here is by detecting literal m.Case in the source code
<whitequark>
otherwise you train people to ignore them
<whitequark>
alright, let's move on to the next one
<whitequark>
right now a Enum's shape is the largest shape that fits every one of the members, considering the sign bit as well
<whitequark>
this works fine if it's a choice of integers, but less so if it's a field with flags, which might well be 32 bits wide with only one (or zero) bits used
<whitequark>
if such an enum becomes e.g. 2-bit wide, issues arise with concatenations, which end up having fields on the wrong positions
<whitequark>
right now we actually warn if a bare integer is used in a concatenation and it's not 0 or 1
<jfng[m]>
could we possibly define our own enum primitive ?
<whitequark>
the same issues arise with the new lib.data library, which is really just a concatenation internally
<whitequark>
jfng: yes, this is one of the options
<jfng[m]>
which would require a width
<whitequark>
in terms of implementation, whatever we do, needs to have the ability to change what Shape.cast(EnumCls) returns for some EnumCls, which is really just a class attribute
<whitequark>
exactly how we do that is up for discussion
<whitequark>
the three obvious options I see are (a) class attribute, (b) decorator, and (c) subclass
<whitequark>
a) class MyEnum(enum.Enum): _amaranth_shape_ = unsigned(16)
<whitequark>
b) @amaranth.enum_shape(unsigned(16)) class MyEnum(enum.Enum):
<whitequark>
c) class MyEnum(amaranth.Enum, shape=unsigned(16))
<jfng[m]>
c) seems more natural, but i'm not familiar with the implementation considerations
<whitequark>
I think it's nice that currently Amaranth can use enums from foreign code
<whitequark>
e.g. you can import libusb and use its enums yourself
<whitequark>
with c) there is a concern that you have four classes: Enum, IntEnum, Flag, IntFlag
<whitequark>
IntEnum is trivial but the rest aren't
<whitequark>
do we provide Amaranth versions of all those?
<whitequark>
oh, there is also option d); allow Const in enum values
<jfng[m]>
why is IntFlag non-trivial ?
<whitequark>
then you can have class B(enum.Enum): FOO = C(2, 0b00) BAR = C(2, 0b01)
<whitequark>
this does mean you lose the ability to use it in non-Amaranth code though
<whitequark>
unless you want to do B.FOO.value everywhere
<whitequark>
jfng: I don't know, I'm looking at the implementation
<whitequark>
it creates pseudo-members with flag combinations on the fly, it seems
<whitequark>
like `<B:FOO|BAR>` or something
<tpw_rules>
is there some metaclass horror amaranth could do if the objects are Consts so that it knows they are but everyone else sees the values?
<jfng[m]>
a minor issue i have with not providing (requiring, even) our own enum, is that it would still let this width mismatch pitfall happen
<whitequark>
tpw_rules: yeah
<jfng[m]>
i agree that transparent compatibility with foreign enums is good though
<whitequark>
tpw_rules: but it's kind of really bad
<whitequark>
worse by far than anything we've shipped
<whitequark>
actually, I misremembered something
<whitequark>
so with class B(enum.Enum): FOO = 0b00, B.FOO is an instance of B
<whitequark>
so, yes, we can ship an enum where B.FOO is a Const for Amaranth, and just a normal B instance for everyone else... which is actually what solutions a), b), and c) are about
<whitequark>
there's already a cast from enum.Enum instances to Const, right?
<whitequark>
the question is just what is the width for the resulting constant
<tpw_rules>
allowing Const to be an enum value is an attractive proposition to me. the question is how to make it a normal value to someone else. i guess also this means how do you annotate someone else's enum with widths?
<whitequark>
this is kind of the same question, since ints are castable to Amaranth values
<whitequark>
we already have the cast that turns 1 into Const(1, shape) when it's in a enum variant, and the shape always matches Shape.cast(E)
<whitequark>
i.e. every enum with int values already implicitly contains Consts
<whitequark>
this issue is about how you define that shape
<tpw_rules>
then would "just stick a const of a specific shape in" be a valid solution?
<tpw_rules>
it sounds like option (d) is just a documentation update then? unless you want to assign a shape to someone else's enum
<whitequark>
that's the idea behind d)
<whitequark>
oh, you currently can't do Shape.cast(E) if E has variants that are Const instances
<tpw_rules>
actually another problem is that it's probably not fun if the Consts have different widths
<whitequark>
this is why you currently can't
<tpw_rules>
then maybe a decorator is the right option
<whitequark>
jfng: one thing in favor of your proposal to subclass enums is that this way, it's possible to detect truncation
<whitequark>
e.g. class E(amaranth.Enum, shape=unsigned(4)): V = 0b10000
<whitequark>
it works with a decorator too
<jfng[m]>
that would be nice
<whitequark>
it does not work with a)
<whitequark>
so I guess a) is probably out, that's a very common issue we should be detecting
<jfng[m]>
with d), having to write C(..., 32) for every member, is a bit noisy for the sake of readability
<whitequark>
it would actually be enough to write it for one for the purposes of shape inference
<whitequark>
but then you have different types inside the enum...
<jfng[m]>
right, but awkward
<jfng[m]>
same as putting a "dummy" value, as a workaround today
<whitequark>
yes
<whitequark>
with the inheritance approach, it is probably possible to make it a mixin
<whitequark>
class E(enum.Flag, amaranth.hdl.EnumMixin, shape=unsigned(32)):
<whitequark>
name TBD
<whitequark>
maybe amaranth.lib.enum.EnumMixin or something, I think it can be a ShapeCastable and then it doesn't have to live in the standard library
<whitequark>
if we have that, we can warn if a normal Python enum is used in a concatenation
<whitequark>
the same as for integers
<whitequark>
normal enums will of course remain shape-castable since there's nothing wrong with doing Signal(E) and that needs to represent the entire range of values of E
<jfng[m]>
should we also have a FlagMixin, to retain flag-specific behavior ?
<whitequark>
the idea with a mixin is that you use enum.Enum, enum.Flag, whatever you want, and then add Amaranth functionality on the side
<jfng[m]>
oh, ok
<jfng[m]>
so E.FOO|E.BAR would still work here
<whitequark>
yeah, Amaranth mixin would be just for the shape
<whitequark>
and for linting
<jfng[m]>
i can't think of something that bothers me
<jfng[m]>
this looks ergonomic, i like it
<jfng[m]>
since enums will have explicit shapes, wouldn't that make the decoder= argument of Signal redundant ?
<whitequark>
you can use any function as a decoder
<whitequark>
put a RISC-V disassembler there
<jfng[m]>
cool, didn't know that
<whitequark>
that's why it's "decoder" and not something like "enum"
<whitequark>
though if you pass an enum it's handled specially