[Moore] Improve declaration and instantiation of SVModule operation

[cepheus69]

The current SVModuleOp does not provide port lists and instanceOp can not establish a connection between SV modules. They completely do not meet growing demand in Moore (circt-verilog frontend) dialect. This PR bring newly improved SVModuleOp and instanceOp. By mature parser and printer provided by HW dialect. We could provide more better IR on basis of those functions. New functions around new operation has been added. Additionally, UnconnectedOp and OutputOp has been added into Moore dialect. The UnconnectedOp denotes unconnected behavior in circuit design. It provides better strategy encountering such a case. (The design refers to IEEE 1800 - 2017 § 23.3.3 Port connection rules) The OutputOp serves as terminator of new SVModuleOp. The original portOp was combined into new SVModuleOp. In this new procedure, basic systemVerilog code could be translated into Moore IR.

[cepheus69]

It is still in testing and developing. The SVModuleOp will be pushed later.

[cepheus69]

The MultiPort code was not included. The logic is added into creating SVModuleOp.

[cepheus69]

The test of InstnaceOp / unconnectedOp / outputOp / SVModuleOp will be added soon after finishing SVModuleOp.

[uenoku]

Do we need to split inputs and outputs? It looks in Moore we pass even output values to an instance op so I feel it's easier to merge them?

[cepheus69]

Because the Moore InstanceOp does not have results. Distinguish the inputs and outputs will be beneficial to the next lowering procedure. Unless we add DirectionAttr into components of the op.

[cepheus69]

The example for instanceOp & UnconnectedOp:

module sub(input a, output b);
endmodule

module top;
   wire i;
   sub sub (.a(), .b(i));
endmodule

To generate:

module {
   ......
  moore.module @top {
    %i = moore.net wire : !moore.logic
    %0 = moore.unconnected In : !moore.logic
    moore.instance "sub" @sub(a: %0: !moore.logic) -> (b: %i: !moore.logic)
  }
}

[cepheus69]

Testing but have problems

[fabianschuiki]

This is interesting. You've mentioned that the updated SVModuleOp will reuse the HW module port list parser and printer, and use a moore.output op terminator. This suggests that the module op will not have arguments for its outputs, but instead use the moore.output terminator to define output values. I like that approach because it makes it clear which SSA values get transported into the module, and which ones get transported back out of the module (and the inout/ref ports will probably just be inputs with a special ref type).

I'm wondering if instances then shouldn't also follow this pattern and mimic what HW does: have a list of operands for all inputs, and a list of results for the outputs. At the moment, the outputs are operands of the instance op. I agree that this matches SV by requiring you to attach a wire to that output in a sense, but it feels like it breaks the pattern established by SVModuleOp where the output actually comes out of the module. I'm curious if and how we could make these two work in the same way.

For example, how could we express the following snippet of Verilog?

module Foo(input int x, output int y);
  assign y = x*2;
  wire int z = y;
endmodule

int a;
Foo foo(.x(42), .y(a));

Using a Terminator and Instance Results 😃

One approach would be moore.module using a moore.output terminator to produce its outputs, as you have suggested. In this case, the values produced as outputs would be defined by the terminator. This also means that the module defines no SSA value for output y, since outputs are not a block argument. I think that's a good thing, it just means that we have to define wires/variables for outputs explicitly such that you can assign to y and read from y in the net declaration.

moore.module @Foo(in %x: !moore.i32, out y: !moore.i32) {
  %y = moore.variable : !moore.i32     // module defines no %y SSA value
  %0 = moore.constant 2 : !moore.i32
  %1 = moore.mul %x, %0 : !moore.i32
  moore.assign %y, %1 : !moore.i32     // assign y = x*2
  %z = moore.net wire %y : !moore.i32  // wire int z = y
  moore.output %y : !moore.i32         // use value of var "y" for output "y"
}

Correspondingly, the instance of Foo should then probably also just accept a bunch of SSA values as input operands, and define a bunch of SSA values as results for its outputs. This means that you cannot just connect a wire/variable to an output port, but instead requires you to use an explicit connect. I think that's a good thing again, and if I remember correctly the Slang AST already contains this assignment.

%a = moore.variable : !moore.i32
%0 = moore.constant 42 : !moore.i32
%1 = moore.instance "foo" @Foo(x: %0: !moore.i32) -> (y: !moore.i32)
moore.assign %a, %1  // assigns output "y" to var "a", making the connection

I really do like this approach. It makes module and instance ports a very lightweight concept in the IR: it's only about transporting an SSA value into the module or out of the module. There are no implicit wires, variables, or connections happing here. All connections are explicitly done as moore.assign in the IR.

Using Block Arguments for Outputs 😞

Another approach would be to make moore.module have a block argument for every single port, including outputs. Inside the module, these block arguments would act as wires or variables, and depending on whether they correspond to an input, inout, output, or ref port, you'd be allowed to do a different set of things. This corresponds pretty much to what Verilog does, but it feels very clunky in an IR since it would introduce a whole bunch of new things besides net/var that you can assign to. And there would be subtle rules regarding which of these block arguments you could assign to (inout and output), and which ones you can only read. Also, this would force us to capture the difference between variables and nets as part of the module port list, since variable and net ports behave differently and may imply an output buffer.

moore.module @Foo(in %x: !moore.i32, out %y: !moore.i32) {
  %0 = moore.constant 2 : !moore.i32
  %1 = moore.mul %x, %0 : !moore.i32
  moore.assign %y, %1 : !moore.i32  // is %y a net or var?
  %z = moore.net wire %y : !moore.i32
}

Instances should probably reflect this pattern and carry all ports as a single list of operands. They would also have to encode the direction of each port, and whether the ports are understood to be wires or nets, and if they contain implicit buffers/drivers that decouple the wires inside the module from the wires attached to the instance.

%a = moore.variable : !moore.i32
%0 = moore.constant 42 : !moore.i32
moore.instance "foo" @Foo(in x: %0: !moore.i32, out y: %a: !moore.i32)

I'm not a big fan of this approach, because it makes ports a very clunky and heavyweight concept in the IR. They define a mapping from instance ports to module ports, but also act as net/var declarations, and they imply assignments and connections. I'd much rather have us define ports as a mapping between SSA values, and then sort out any SystemVerilog connectivity and semantics via explicit IR ops in the module and the instance.

What do you think @uenoku, @cepheus69, @hailongSun2000, @maerhart?

[hailongSun2000]

For module, I keep the previous thought. Don't make the output y as part of the block argument, instead of using wire/variable to declare it. Therefore, we needn't care for the Port connection rules for nets/variables, and just focus on the inner symbol(wire/variable) used as the output y.

---

For instance, I use Slang to test your example:

{
                  "port": "2199025174904 y",
                  "expr": {
                    "kind": "Assignment",
                    "type": "int",
                    "left": {
                      "kind": "NamedValue",
                      "type": "int",
                      "symbol": "2199025172832 a"
                    },
                    "right": {
                      "kind": "EmptyArgument",
                      "type": "int"
                    },
                    "isNonBlocking": false
                  }
                }

You're totally right! We can capture the output y and SSA %a, then create moore.assign %a, %1 by establishing the map between y and %a. And we can ignore the port direction. I don't have a good handle on the further details right now.

I agree with Using a Terminator and Instance Results.

[cepheus69]

Agree with the first design. The reason why we should use a terminator with net / variable declaration instead of block arguments: We should not eliminate variable/net logic in the Moore layer. It will make the conversion procedure more complicated. Removing the logic too early will cause problems in analyzing and transforming passes at the Moore level. This design actually minimizes the coupling of SVModuleOp (initially portOp) to the net / variable logic. because of preserving the net / variable logic, instanceOp has no result but attaches the original logic in the SV module via net / variable operation, etc.

[maerhart]

@fabianschuiki I agree with you and am in favor of the terminator and instance results approach since it is easier to analyze and optimize. We need to be sure that we can model SV semantics with that approach, though. I'm not familiar enough with all the details in the standard to tell that there isn't a "feature" that could have some issues with this way of modeling things (but I'd guess most likely not, I think they are equally powerful in expressiveness).

Obviously, we can't get rid of connect semantics entirely and modeling instances this way means we push the variable/net declaration to the leaf modules and pass them bottom-up instead of top-down, which makes sense since it minimizes the number of wire connects we need to model in a typical design.

Some pseudocode as an illustration (let me know if there are any conceptional issues with it):

module A(output a);
read a
drive a
endmodule

module B(output b);
inst a1 A(b)
inst a2 A(b)
read b
endmodule

module C(input c);
read c
endmodule

module top();
wire d
inst b B(d)
inst c C(d)
endmodule

moore.module @A(out a) {
  %0 = moore.variable
  read %0
  drive %0
  moore.output %0 
}
moore.module @B(out b) {
  %0 = moore.instance @A()
  %1 = moore.instance @A()
  %2 = connect %0, %1
  read %2
  moore.output %2
}
moore.module @top() {
  %0 = moore.instance @B()
  moore.instance @C(%0)
  moore.output
}

They would also have to encode the direction of each port, and whether the ports are understood to be wires or nets, and if they contain implicit buffers/drivers that decouple the wires inside the module from the wires attached to the instance.

Do you have a reference to the part in the standard that talks about the implicit buffers and drivers? Why is this not a concern in the other approach as well? They seem equivalent to me w.r.t that.

[hailongSun2000]

I suddenly remember a case--BCD_Adder--it also exists the wire connections like w1, w2, and w3. And output [3:0] Sum is extracted as different bits to be used for the different instantiations.

module FullAdder (
    input A, B, Cin,
    output Sum, Cout );

    assign Sum = A ^ B ^ Cin;
    assign Cout = A & B | A & Cin | B & Cin;

endmodule

module BCD_Adder_4bit (
  input [3:0] A, B,
  input Cin,
  output [3:0] Sum,
  output Cout );

  wire w1, w2, w3, w4;
  FullAdder fa1 (.A(A[0]), .B(B[0]), .Cin(Cin), .Sum(Sum[0]), .Cout(w1));
  FullAdder fa2 (.A(A[1]), .B(B[1]), .Cin(w1), .Sum(Sum[1]), .Cout(w2));
  FullAdder fa3 (.A(A[2]), .B(B[2]), .Cin(w2), .Sum(Sum[2]), .Cout(w3));
  FullAdder fa4 (.A(A[3]), .B(B[3]), .Cin(w3), .Sum(Sum[3]), .Cout(w4));

  assign Cout = w4;

endmodule

[cepheus69]

It looks like:

moore.module @A(out a : !moore.xxxx) {
  %a = moore.variable "a" : !moore.xxxx
  moore.output %a : !moore.xxxx
}
moore.module @B(out b : !moore.xxxx) {
  %b = moore.net wire : !moore.xxxx
  moore.instance "xxxx" @A() -> (a: %b: !moore.xxxx)
  moore.instance "xxxx" @A() -> (a: %b: !moore.xxxx)
  moore.output %b : !moore.xxxx
}
moore.module @top() {
  %0 = moore.net wire : !moore.xxxx
  moore.instance "xxxx" @B(c: %0: !moore.xxxx) -> ()
  moore.instance "xxxx" @C(c: %0: !moore.xxxx) -> ()
  moore.output
}

It indeed needs var or net to express the semantics of the connection.

[fabianschuiki]

@maerhart Do you have a reference to the part in the standard that talks about the implicit buffers and drivers? Why is this not a concern in the other approach as well? They seem equivalent to me w.r.t that.

I was thinking about the convoluted rules in "23.2.2.3 Rules for determining port kind, data type, and direction". There's a distinction between wire and variable ports. If I recall correctly, variable ports imply a buffer that decouples the module internals from the outside. If you have a wire input port, the wire connected to the instance and the wire inside the module will be the same object in the event queue, and if you force a value onto that input inside the module (why you'd do that I don't know 😬), that value would also show up at the instantiation site. If you have a variable input port however, there is an implied buffer gate that reads the value of the wire connected to the instance and drives it onto the variable inside the module. But the two are distinct entities; if you force the variable port inside the module, the buffer prevents that force from propagating to the instance site.

You're totally right, this is a concern in both approaches. But if we use instance results and a terminator op we'd explicitly create these buffers in the IR. If the ports were all wire-style operands, these subtle differences between port types/kinds would be part of SVModuleOp.

[fabianschuiki]

@maerhart I think in your example we'd create explicit variable/net ops in the IR to represent the ports (and whether the ports are variables or nets). And then have explicit assigns at the instantiation sites to capture how the values coming out of the instance as results get driven onto the connected variables/nets. The instance only copies the value out of the module body, and the details of the connects done by the instance are handled next to the instance explicitly:

moore.module @A(out a) {
  %0 = moore.variable
  read %0
  drive %0
  moore.output %0 
}
moore.module @B(out b) {
  %0 = moore.variable
  %1 = moore.instance @A()
  %2 = moore.instance @A()
  moore.assign %0, %1 // multi-driver -- resolved in a pass?
  moore.assign %0, %2 // multi-driver -- resolved in a pass?
  read %0
  moore.output %0
}
moore.module @top() {
  %0 = moore.variable
  %1 = moore.instance @B()
  moore.assign %0, %1
  moore.instance @C(%1)
  moore.output
}

Your example is great because it also contains a multiply-driven variable in module "@b". Having the assignments explicitly in the IR is pretty cool because we could push the handling of multiple drivers on variables and nets into its own pass.

Mem2reg could then get rid of quite a few of the explicit variables and nets that we insert for instances.

[fabianschuiki]

@hailongSun2000 In your example, does the Slang AST contain assignment expressions for things like .Sum(Sum[0])? I'd expect something like (pseudo-AST):

port: "Sum",
expr: Assignment {
  left: Index {
    expr: NamedValue("Sum"),
    index: 0,
  },
  right: EmptyArgument,
}

If that's the case, this would pretty trivially expand to an IR like:

%Sum = moore.variable : l4
%w1 = moore.net wire : l1
// ...
%fa1.Sum, %fa1.Cout = moore.instance "fa1" @FullAdder (
  A: %a0: l1, B: %b0: l1, Cin: %Cin: l1) -> (Sum: l1, Cout: l1)
%0 = moore.extract %Sum, 0 : l4   // Sum[0]
moore.assign %0, %fa1.Sum : l4    // .Sum(Sum[0])
moore.assign %w1, %fa1.Cout : l4  // .Cout(w1)
// ...

[hailongSun2000]

@fabianschuiki. I'll show you the related Slang AST tomorrow(I have off today). But I can promise that we can capture the sum[0] like you expect.

[hailongSun2000]

Here is the related AST:

{
                  "port": "2199025212832 Sum",
                  "expr": {
                    "kind": "Assignment",
                    "type": "logic",
                    "left": {
                      "kind": "ElementSelect",
                      "type": "logic",
                      "value": {
                        "kind": "NamedValue",
                        "type": "logic[3:0]",
                        "symbol": "2199025203920 Sum"
                      },
                      "selector": {
                        "kind": "IntegerLiteral",
                        "type": "int",
                        "value": "0",
                        "constant": "0"
                      }
                    },
                    "right": {
                      "kind": "EmptyArgument",
                      "type": "logic"
                    },
                    "isNonBlocking": false
                  }
                },

We can capture the .Sum(Sum[0]), but maybe do we have to concate Sum[0], Sum[1], ... right? At the moore level, maybe we can directly output Sum[3:0], like moore.output %Sum[3:0], %Cout. However, when convert-moore-to-core, the assignments will be erased, and for the SSA Sum[3:0], we need to connect the %fa1.sum, ..., and %fa4.sum together which will be assigned to Sum[3:0]. For example:

hw.module @BCD...
  %Sum = hw.wire %1 : i4;
  %Cout = hw.wire %fa4.Cout : i1;
  %fa1.Sum, %fa1.Cout = moore.instance "fa1" @FullAdder (
  A: %a0: l1, B: %b0: l1, Cin: %Cin: l1) -> (Sum: l1, Cout: l1)
  %fa2.Sum, %fa2.Cout = ...
  %fa3.Sum, %fa3.Cout = ...
  %fa4.Sum, %fa4.Cout = moore.instance "fa4" @FullAdder (
  A: %a3: l1, B: %b3: l1, Cin: %Cin: l1) -> (Sum: l1, Cout: l1)
  %1 = comb.concat %fa1.Sum, %fa2.Sum, %fa3.Sum, %fa4.Sum : i4
  hw.output %Sum, %Cout

[fabianschuiki]

I think you're right, at some point we'll probably need to add a concatenation to have a single %Sum value to return. I would expect the mem2reg pass to do most of this: there is a related op interface defined by mem2reg that talks about structs/arrays that can be indexed into (e.g. as Sum[0]) and then assigned. If have no idea how exactly that works. And there are probably corner cases where we can't do the concatenation, for example, if a variable is assigned from different processes in non-synthesizable parts of the design. I'm pretty sure we'll be able to handle those corner cases later by repurposing the LLHD dialect a bit, and turning it more into an event queue dialect that allows us to represent all the weird things in Verilog which the standard core dialects can't fully represent.

[fabianschuiki]

@hailongSun2000 Here is the related AST: ...

That's very exciting! It looks like Slang already has all these assignments that we need if instances simply move values in and out of modules. Feels like we're onto a nice design here 😃