netlist handling of multiple blocking assignments of same var in always_comb

[udif]

@jameshanlon
Describe the bug
It seems that when a variable is reassigned multiple times in a blocking assignment inside a combinatorial always block, all assigments are treated as the same node.

To Reproduce
Please produce a graph from this small program:

module t2 (input clk, output reg [31:0]nq);
reg [31:0]n;
always_comb begin
        n = nq;
        n = n + 1;
end

always_ff @(posedge clk)
        nq <= n;
endmodule

[jameshanlon]

This is because there is no logic to handle multiple assignments to the same variable in a procedural block. I'll take a look at adding it.

[jameshanlon]

I've been having a look at this issue and have come to the conclusion that it's not possible to implement the correct handling of variables in procedural blocks without performing dataflow analysis to determine the reaching definitions. This analysis is also complicated by the way parts of variables can be selected by ranges etc. So it would be a significant development of the tool to do this and beyond the scope of a bug fix or incremental feature.

@MikePopoloski I wonder if you have any thoughts on how to approach this. I know you have mentioned before plans of lowering the AST into an MLIR-based form that would be more amenable to data flow analyses. If this is the case, then it would make sense to align with that.

For the time being, I think this should be marked as a feature request rather than a bug.

[MikePopoloski]

Ok, that's fine.

Yes, I'm hoping to be able to start looking at an MLIR layer this summer, precisely to get all of their included data flow analysis stuff for free instead of trying to reimplement it on top of the slang AST (which would be doable but clunky). There's already an effort underway to have the slang AST translated to MLIR as part of the CIRCT project, so I'll see if there's some useful overlap there.

[udif]

@jameshanlon, I noticed your development branch, but haven't actually tried it yet.
I was wondering what are the specific test cases that fails and lead you to your conclusion.
I took a sample case, which your original code seem to be handling fine, even before you started your branch:

module t35;
reg [4:0]a;
always_comb
begin
        for (int i = 1; i < 5; i++)
                a[i] = a[i-1];
        a[0] = a[4];
end
endmodule

You also handle struct members (test not included here).

Handling if (and probably case) is not 100% at the moment.

module t37;
reg a, b, c;
always_comb
begin
  if (a)
          b = c;
  else
          b = a;
  c = a;
end
endmodule

I assume that all the hierarchical constructs cause no problems, and that the current issues are only with always processes.
I was looking at what subset of features needs to be supported for a useful implementation for those (by "useful" I mean handling code you expect to find in synthesizable RTL). Anything not falling under "useful" you can either warn against and quit:

• deterministic for loops (e.g. based on constant values)
• if and case statements
• no break or continue (or perhaps later yes but only if it is conditioned by constants such as loop counters or parameters (e.g. i < PARAM)
• structs
• indexed arrays and bit ranges

I have never used LLVM or MLIR myself, so I don't know how complex would it be to use them, but I was wondering if building an in-memory flattened SystemVerilog representation of the entire process, using made-up register names for splitting blcking assignment variables, struct fields (only if not supported yet - I haven't checked) and then compile it using a separate compilation object and a separate tree walker for that AST would provide any benefit.

[jameshanlon]

I was wondering what are the specific test cases that fails and lead you to your conclusion.

There are several aspects that I can currently see with dependencies in procedural blocks:

1. Dependencies between multiple assignments to the same variable, where each assignment can specify a particular range.
2. Handling depencencies in branching and reconverging control flow (ie if and case).
3. Handling dependencies between variables used for conditional control.

It is true that the tool can properly handle ranged assignments and selections (thanks to slang's value driver logic), and this can be used to resolve multiple assignments (such as in the example on this issue). But the whole problem gets much more complicated when combined with control flow. Regarding point three, I previously made an adhoc attempt to resolve the third point for if statements in #781, but something more general is needed to handle the different constructs.

I assume that all the hierarchical constructs cause no problems, and that the current issues are only with always processes.

Yes, it is only the procedural always blocks that permit arbitrary combination of statements and assignments.

I was looking at what subset of features needs to be supported for a useful implementation for those (by "useful" I mean handling code you expect to find in synthesizable RTL).

I like this idea, and a 'netlist' tool will always be focussed on Verilog for synthesis so it makes sense that the feature set can be sensibly constrained. However, in my view if and case and their combinations in procedural blocks are important in synthesizable RTL.

I think my position is to say (for the time being at least) that the tool can only handle procedural blocks with a single assignment to a particular variable (or bit range of a variable). That sidesteps points 1 and 2. Point three can be addressed with more ad-hoc support. My old tool netlist paths was useful with this restriction, on top of there being no support for accessing sub components of variables.