e.g.: s = SimdSignal(...) Cat(Const(...), s) Currently nmigen expects the first argument to always be a SimdSignal: https://git.libre-soc.org/?p=nmigen.git;a=blob;f=nmigen/hdl/ast.py;h=39fb33f4d7bd36b33936c5e9f3f962aed7532150;hb=refs/heads/libresoc-partsig#l870 870 # assume first item defines the "handling" for all others 871 first, rest = args[0], args[1:] 872 return first.__Cat__(*rest, src_loc_at=src_loc_at) we should emulate Python's __add__ method selection logic, calling __Cat__ on the farthest subclass in the input arguments. See note in docs: https://docs.python.org/3/reference/datamodel.html#object.__ror__ > If the right operand’s type is a subclass of the left operand’s type and that subclass provides a different implementation of the reflected method for the operation, this method will be called before the left operand’s non-reflected method. This behavior allows subclasses to override their ancestors’ operations.
"Note If the right operand’s type is a subclass of the left operand’s type and that subclass provides a different implementation of the reflected method for the operation, this method will be called before the left operand’s non-reflected method. This behavior allows subclasses to override their ancestors’ operations." ngghhh.... yyeah that makes sense. i always wondered how that worked. does it apply to Cat() though? i really don't know. i made an arbitrary choice: however... whatever the choice is, they are all going to be "bad" s = SimdSignal(...) Cat(Const(...), s) or Cat(Const(...), s) these are both "valid", so which one should be acceptable? which one prohibited, the other allowed? i believe my thinking here was (without documenting it), that *all* of Cat()'s arguments are expected to be SimdSignals. or, at least, if the first one is, everything else follows. a more advanced version would detect the type of non-SimdSignal instances within the list and upcast them. however what i didn't want to do was have that logic "pollute" nmigen.ast.Cat with things that are SimdSignal-specific. it's a tricky one, i'm not sure what the best solution is here, and was going for "wait and see what emerges down the line" when we have a lot more info about how this works in practice. one obvious workaround: a SimdScope.Cat(). used as: with SimdScope(....) as s: .... s.Cat(...) however it is... hmmm... it would be the first major ast Operator that fell into this usage pattern, it has implications. really don't know, here.
as in, rather than: s = SimdSignal(...) Cat(Const(...), s) it would be e.g.: s = SimdSignal(...) c = SimdSignal(...) comb += c.eq(Const(...)) Cat(c, s) which "avoids" the problem that i think might turn out to be unique to Cat()
(In reply to Luke Kenneth Casson Leighton from comment #2) > as in, rather than: > > s = SimdSignal(...) > Cat(Const(...), s) > > it would be e.g.: > s = SimdSignal(...) > c = SimdSignal(...) > comb += c.eq(Const(...)) > Cat(c, s) > > which "avoids" the problem that i think might turn out > to be unique to Cat() nope, it also occurs in Mux and Part. s1 = Signal() s2 = SimdSignal() Mux(s1, Const(), s2) Part(Const(), s2, 3, 5) For Cat, we look for the most-derived subclass and use that. more when I have time.
mmm... __rCat__ hmmm... makes me slightly nervous that because of the multiple arguments it doesn't quite fit the pattern created by python x.__rXXX___ which seems to hsve been designed for 2-arg operations. still not a real clue here.
thinking of Cat(a, b, c, d, e, f) -> c.__Cat__((a, b), (d, e, f))
(In reply to Jacob Lifshay from comment #3) > (In reply to Luke Kenneth Casson Leighton from comment #2) > > as in, rather than: > > > > s = SimdSignal(...) > > Cat(Const(...), s) > > > > it would be e.g.: > > s = SimdSignal(...) > > c = SimdSignal(...) > > comb += c.eq(Const(...)) > > Cat(c, s) > > > > which "avoids" the problem that i think might turn out > > to be unique to Cat() > > nope, it also occurs in Mux and Part. ok. > s1 = Signal() > s2 = SimdSignal() > Mux(s1, Const(), s2) yeah this one i went with the rule "selector is the priority". > Part(Const(), s2, 3, 5) > > For Cat, we look for the most-derived subclass and use that. ahh that makes sense. except... oh i know. a static SimdSignal.Cat. find the most-derived class then call the static function with the complete list. def Cat(*arglist) kls = find_most_derived_class(arglist) kls.Cat(*arglist) > more when I > have time. ok. these are things i am happy to go with a workaround for the first version, in order to keep the patch to nmigen to a bare minimum. the last thing we need right now is for that 400 line diff to turn into 4,000. also, remember, we need to get off critical path so if work can proceed on the ALUs with workarounds such as converting Consts to SimdSignal, and not using mixed Signal and SimdSignal, *great*. the ALU conversion gives us data about what the priorities are: what matters and what doesn't pragmatic.
https://www.programiz.com/python-programming/methods/built-in/issubclass walk the list (or other params), call issubclass. made slightly more complex by the fact that ValueCastable is not part of the Value chain, in fact it throws a royal spanner in the works. a partial hack may be possible with: "if isinstance(1st, ValueCastable) and not isinstance(2nd, ValueCastable) highest = 1st" and proceed from there