Compatibility mode for mixed continuous and procedural assignments

[jrudess]

The module instantiated by the generate-loop only drives field 'a' of the struct. The for-loop within the always_comb block only drives field 'b' of the struct. I had to copy the original RTL organization almost exactly to reproduce this one, and simpler attempts to reproduce were passing.

module m(output logic a);
    always_comb a = 0;
endmodule

module top;

    typedef struct packed {
        logic a;
        logic b;
    } s;

    s s1[2];

    always_comb begin
        for (int p=0; p < 2; p++) begin
            s1[p].b = 0;
        end
    end

    for (genvar i = 0; i < 2; i++) begin
        m m1(.a(s1[i].a));
    end

endmodule

langtest67.sv:21:17: error: cannot mix continuous and procedural assignments to variable 's1'
        m m1(.a(s1[i].a));
                ^~~~~~~
slangtest67.sv:16:13: note: also assigned here
            s1[p].b = 0;
            ^~~~~~~

[MikePopoloski]

It's a little confusing but I think slang is correct here. The rule from the spec is:

The precise rule is that it shall be an error to have multiple continuous assignments or a mixture of procedural and continuous assignments writing to any term in the expansion of a written longest static prefix of a variable (see 11.5.3 for the definition of a longest static prefix).

The longest static prefix within the always_comb is simply s1 as the element select is not a constant expression (even though you can look at it and infer that it will always be one of two values), and so we never make it down to the field select to see that only one field is ever assigned.

It looks like Questa will correctly diagnose this case; other tools do not. I suspect they're doing some loop unrolling as an optimization prior to enforcing this rule and so do not see it.

[jrudess]

If we avoid using arrays, then the same driver conflict doesn't exist. It seems odd that use of arrays would make the language more restrictive for the same struct-type. I will have to investigate if we can use packed-arrays and whether the same issue exists with those.

E.g. this passes.

module m(output logic a);
    always_comb a = 0;
endmodule

module top;

    typedef struct packed {
        logic a;
        logic b;
    } s;

    s s1;

    always_comb begin
        s1.b = 0;
    end

    m m1(.a(s1.a));

endmodule

[jrudess]

Ah, I understand this better now. Agree that this is not a bug, and we can resolve by moving the always_comb into the generate so that the indexing is static.

[jrudess]

Getting some internal push-back that Questasim might be too restrictive here based on this wording in 11.5.3:

Informally, the longest static prefix of a select is the longest part of the select for which an analysis tool has known values following elaboration. This concept is used when describing implicit sensitivity lists (see 9.2.2.2) and when describing error conditions for drivers of logic ports (see 6.5). The remainder of this subclause defines what constitutes the "longest static prefix" of a select.

So I still don't think Slang violates the LRM here, because it is is the analysis-tool choice whether it implements loop unrolling. How feasible is it to understand loops that have constant boundaries vs loops that do not, so that the 'select' could be handled as static for more scenarios?

e.g.

for (int i = 0; i < 4; i++)

vs

for (int i = 0; i < some_other_signal; i++)

[MikePopoloski]

It's not really a choice; that paragraph is the "informal" definition. The formal definition is given right after, and for a select:

An indexing select is a static prefix if the indexing select prefix is a static prefix and the select expression is a constant expression.

A constant expression is a well defined term, so this clearly does not apply.

As to your question, it's probably doable to special case this one kind of loop, but it's pretty weird. Like, what if the loop has multiple variable declarations? What if it doesn't declare its iteration variable at all, or refers to a previously declared variable that may or may not be assigned a constant value at some point? What if it's assigned conditionally? What if it's a while loop instead, or a foreach loop? I'm a little worried that this basically simplifies to solving the halting problem. Special casing for optimization purposes is great, special casing for language rules makes them very hard to follow I think, which is probably why the LRM defines it this way, and at the end of the day the code still won't be compatible with all other tools out there that stick to the LRM rules.

[MikePopoloski]

Also I wouldn't be surprised if you could break the other tools by making your loop more complicated, such that the optimizer decides it's no longer worthwhile to unroll it, and suddenly your assignment that previously worked throws an error due to your loop being more complicated.

[jrudess]

Thanks, I concur that the spec is pretty clear here, and agree that minor tweaks on the loop would likely get rejected by VCS.

The unfortunate situation is that it is silicon proven code, and it does appear that a large majority of the EDA tools are accepting it (DesignCompiler, JasperGold, and even Mentor's own 0-In CDC), so the RTL design owners understandably don't want to touch it.

[MikePopoloski]

I think it would be fair to have a compatibility flag to support this since other tools do. I'm not entirely sure how I'd go about it but it's of course achievable with enough effort.

[MikePopoloski]

I believe this is in a good state now.