Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Semantic error checker flags non-real errors #739

Closed
nblei opened this issue May 28, 2024 · 3 comments · Fixed by #740
Closed

Semantic error checker flags non-real errors #739

nblei opened this issue May 28, 2024 · 3 comments · Fixed by #740
Labels
bug Something isn't working

Comments

@nblei
Copy link
Contributor

nblei commented May 28, 2024

The check for multiple assignments occurs at the variable level, rather than the bit/wire level.

module fa (
    i_a : input  logic,
    i_b : input  logic,
    i_ci: input  logic,
    o_s : output logic,
    o_co: output logic,
) {
    assign o_s  = ^{i_a, i_b, i_ci};
    assign o_co = |{i_a & i_b, i_a & i_ci, i_b & i_ci};
}

module rca #(
    param WIDTH: u32 = 8,
) (
    i_a : input  logic<WIDTH>,
    i_b : input  logic<WIDTH>,
    i_ci: input  logic       ,
    o_s : output logic<WIDTH>,
    o_co: output logic       ,
) {
    var carries: logic [WIDTH + 1];

    assign carries[0] = i_ci;
    assign o_co       = carries[WIDTH];

    for i in 0..WIDTH :gen_fas {
        inst u_fa: fa (
            i_a : i_a[i]        ,
            i_b : i_b[i]        ,
            i_ci: carries[i]    ,
            o_co: carries[i + 1],
            o_s : o_s[i]        ,
        );
    }
}
  × carries is assigned in multiple procedural blocks or assignment statements
    ╭─[/home/nathan/Documents/Reviews/MICRO-2024/rca/src/rca.veryl:20:1]
 20 │ ) {
 21 │     var carries: logic [WIDTH + 1];
    ·         ───┬───
    ·            ╰── Error location
 22 │ 
 23 │     assign carries[0] = i_ci;
    ·     ───┬──
    ·        ╰── Assigned
 24 │     assign o_co       = carries[WIDTH];
 25 │ 
 26 │     for i in 0..WIDTH :gen_fas {
 27 │         inst u_fa: fa (
 28 │             i_a : i_a[i]        ,
 29 │             i_b : i_b[i]        ,
 30 │             i_ci: carries[i]    ,
 31 │             o_co: carries[i + 1],
    ·             ──┬─
    ·               ╰── Assigned too
 32 │             o_s : o_s[i]        ,
    ╰────
@dalance dalance added the bug Something isn't working label May 29, 2024
@dalance
Copy link
Collaborator

dalance commented May 29, 2024

Partial assignment at port connection seems to be treated as full assignment now.
I'll fix it.

@nblei
Copy link
Contributor Author

nblei commented May 31, 2024

Naoya,

This seems to have been a course correction too far!

module rca #(
    param WIDTH: u32 = 8,
) (
    i_a : input  logic<WIDTH>,
    i_b : input  logic<WIDTH>,
    i_ci: input  logic       ,
    o_s : output logic<WIDTH>,
    o_co: output logic       ,
) {
    // carry-outs
    var carries: logic [WIDTH + 1];

    assign carries[0] = i_ci;
    assign carries[0] = ~i_ci;
    assign o_co       = carries[WIDTH];

    for i in 0..WIDTH :gen_fas {
        inst u_fa: full_adder (
            i_a : i_a[i]        ,
            i_b : i_b[i]        ,
            i_ci: carries[i]    ,
            o_co: carries[i + 1],
            o_s : o_s[i]        ,
        );
    }
}

Now this builds successfully, despite having multiple assignments to the same bit. This is, I think, better than before, because the bug will get picked up during synthesis, but is still not ideal.

@dalance
Copy link
Collaborator

dalance commented May 31, 2024

Yes. The multiple assignment by partial assignment is not checked yet.
I created a new issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants