Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Default assignments for non-@data ports #1919

Closed
rachitnigam opened this issue Feb 16, 2024 · 5 comments · Fixed by #1610
Closed

Default assignments for non-@data ports #1919

rachitnigam opened this issue Feb 16, 2024 · 5 comments · Fixed by #1610
Labels
C: Calyx Extension or change to the Calyx IL Type: Bug Bug in the implementation

Comments

@rachitnigam
Copy link
Contributor

If there are no assignments to a non-@data port, the compiler will fail to generate a default assignment for that port leading to that control-path value being set to 'x. The following example on #1607 fails because of this:

import "primitives/core.futil";
import "primitives/memories/seq.futil";

component main() -> () {
    cells {
        @external m = seq_mem_d1(32, 2, 2);
        @external o = seq_mem_d1(32, 2, 2);
        r = std_reg(32);
        add = std_add(32);
    }
    wires {
        static<2> group read_m {
            m.addr0 = 2'd1;
            m.content_en = %[0:1] ? 1'd1;
            add.left = m.read_data;
            add.right = 32'd1;
            r.in = add.out;
            r.write_en = %[1:2] ? 1'd1;
        }
        static<1> group write_m {
            o.addr0 = 2'd1;
            o.content_en = %[0:1] ? 1'd1;
            o.write_en = %[0:1] ? 1'd1;
            o.write_data = r.out;
        }
    }
    control {
        read_m; write_m;
    }
}

Data file:

{
    "m": {
        "data": [
            10,
            20
        ],
        "format": {
            "numeric_type": "bitnum",
            "is_signed": false,
            "width": 32
        }
    },
    "o": {
        "data": [
            0,
            0
        ],
        "format": {
            "numeric_type": "bitnum",
            "is_signed": false,
            "width": 32
        }
    }
}

Expected output:

{
  "cycles": 3,
  "memories": {
    "m": [
      10,
      20
    ],
    "o": [
      0,
      21
    ]
}

Actual output:

{
  "cycles": 3,
  "memories": {
    "m": [
      10,
      20
    ],
    "o": [
      0,
      1
    ]
}

The problem is that because write_en is set to 'x, the read m[1] returns 0 instead of 20.

The solution is adding a compilation pass that generates default assignments to non-@data ports with no source-level assignments. It's a little tricky to figure out where exactly this pass should go since adding it at the start of the compilation will break dead-cell-removal and cell-sharing passes.

@rachitnigam rachitnigam added Type: Bug Bug in the implementation C: Calyx Extension or change to the Calyx IL labels Feb 16, 2024
@rachitnigam rachitnigam linked a pull request Feb 18, 2024 that will close this issue
@rachitnigam
Copy link
Contributor Author

Relevant to @calyxir/semantics!

@EclecticGriffin
Copy link
Collaborator

Was this supposed to be closed?

@rachitnigam
Copy link
Contributor Author

The pass adds these assignments automatically but we should discuss what the right thing to do semantically is

@EclecticGriffin
Copy link
Collaborator

Ah gotcha

@sampsyo
Copy link
Contributor

sampsyo commented Feb 27, 2024

Fascinating! Yes, it seems like this is relevant to a somewhat larger discussion (which does not have an issue yet, IIRC) about moving default-0 assignments from the Verilog backend to an actual, proper Calyx pass. The advantages would be (1) the semantics are clearer, (2) the 0-assignment insertion could be shared between the Verilog and FIRRTL backends instead of reimplemented separately by both, and (3) we would have a chance to optimize them away ourselves opportunistically.

Roughly speaking, you can imagine making this work by taking each non-@data port, creating a default-0 assignment for it, and then converting it into a @data port to indicate that it no longer needs any implicit fallbacks. Then it can be treated like any other @data port for the rest of compilation.

In the long term, you can even imagine flipping the defaults, so ports must be annotated with @control to get default-0ing generated for them. Then the pass would be a "@control elimination" pass. But that is obviously harder for BC reasons.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C: Calyx Extension or change to the Calyx IL Type: Bug Bug in the implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants