Skip to content

demux component bargraph mode never turns on highest output #3519

Open

Description

@pcw-mesa

Not an earthshaking issue, but...

demux works as expected in normal (one-hot) mode
but does not turn on the last output in bargraph mode

(tested with personality=8)

Also the man page states:

   demux.N.out-MM bit out (MM=00..personality)
          The set of output bits

But the output bits are 00..personality-1

Activity

  1. BsAtHome commented on Jul 19, 2025

    @BsAtHome
    Contributor

    With personality=8, you get pins out-00 through out-07, which are 8 pins. It should read (MM=00..personality-1) in the man-page.

    That the last bit is not turned on is a logic error in the code.

    The bargraph has personality+1 states ("0=all off" through "personality=all on") whereas the demux version only has personality states. In your test case: 0 for none activated and 8 for all activated, thus 0...8 and that equals 9 possible values.

    That also makes the use of sel-bit-XX problematic with personality=32 because it cannot represent that case. You can use the sel-u32 pin to set the value.

    This is the snippet:

        if (bit >= personality) bit = personality - 1;
        for (i = 0; i < personality ; i++) {
            out(i) = (bargraph) ? (bit > i) : (bit == i);
        }

    It may be changed into:

      if(bargraph) {
        for (i = 0; i < personality; i++) {
          out(i) = bit > i;
        }
      } else {
        if (bit >= personality) bit = personality - 1;
        for (i = 0; i < personality; i++) {
          out(i) = bit == i;
        }
      }

    Another reason for removing the ternary inner test on bargraph is because it is a shared memory variable (parameter), which must be physically read each time in the loop. But that 1) creates a race because the variable can change in the execution of the loop and 2) if it doesn't change, it is ineffective because it cannot be optimized by the compiler.

  2. rmu75 commented on Jan 23, 2026

    @rmu75
    Collaborator

    Fixing the manpage is a no-brainer. The question is: can something go wrong if we fix the bargraph mode? Or leave it like that and just document the shortcoming?

  3. andypugh commented on Jan 23, 2026

    @andypugh
    Collaborator

    I think it is probably safe to change it. Whilst we can't know what the component is being used for (ie, can't assume that it only drives LEDs and not guillotines, for example) allowing the (presumably unused in existing systems) top output to set is unlikely to have any affect at all.

  4. hansu commented on Apr 11, 2026

    @hansu
    Member

    What I'd like to mention here is that the name doesn't really match the function.
    It is more a bit shift than a demultiplexer.
    It could be interpreted as bit demultiplexer with fixed input.
    I would like to rename this component but this will probably break a lot of configs.
    We could add an optional input pin, so that it is a real demultiplexer but stays compatible. But a bit demultiplexer makes not much sense compared to the current function. Hmm...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions