[Moore][Dedup] Dedup module op

[mingzheTerapines]

#7207 This PR can only dedup same name module OP under same module scope generating which is more lightweight.
This PR can dedup all equipllance module OP which is more powerful. This is a pass.
They are two different methods of deup.

[mingzheTerapines]

@fabianschuiki @hailongSun2000 @cepheus69 @angelzzzzz This is a initial architecture for dedup Pass in Moore. Please have a look and give some advice, I will aprreciate it. =)
More attributes or value will be added for uniqueness definition.

[fabianschuiki]

The big challenge for deduplication passes is that they need to compare the entire module, including all the operations contained inside of them. And you would want to do this starting in the leaf modules and work your way up to the parents. (Because deduplicating leaf modules opens new deduplication opportunities in parent modules.)

Take a look at the FIRRTL dedup pass, or the Arc dedup pass, for some inspiration. You need something like a hasher and a structural equality checker that can traverse the body of two modules to check whether they are the same.

Only comparing the port types is dangeorus, because it would deduplicate @Add(in i4, in i4, out i4) and @Mul(in i4, in i4, out i4), even though the two modules do very different things 😃

[fabianschuiki]

It would be great to not create duplicate IR for instances of the same module! There was an earlier discussion with Mike Popolski about the topic: MikePopoloski/slang#670. He pointed me at a commit that removed instance caching from Slang. I'm pretty sure we can add a similar kind of caching to ImportVerilog, where we first check whether we have already converted an exact version of the module and reuse that instead of creating a new one. You could probably create a wrapper struct around a Slang module that computes a hash and compares two modules for structural equivalence, and then use that to intern modules into a DenseMap. That would allow us to not even create duplicated IR in the first place 😄

[mingzheTerapines]

You are right. I may remove this.

[hailongSun2000]

Hey, @fabianschuiki! My understanding is when we translate from slang AST to moore IR, we don't create the duplicated module for the instance, right? Like:

module Top;
    Foo foo1;
    Foo foo2;
endmodule

module Foo;
endmodule

moore.module @Top();
  moore.instance "foo1" @Foo;
  moore.instance "foo2" @Foo;     // Previously, there is moore.instance "foo2" @Foo_0;
endmodule

moore.module @Foo();
endmodule

And from my perspective, the workflow of moore IR is
1.

moore.module Top();
endmodule

moore.module Top();
endmodule

moore.module Foo();
endmodule

moore.module Top();
  moore.instance "foo1" @Foo;
endmodule

moore.module Foo();
endmodule

moore.module @Top();
  moore.instance "foo1" @Foo;
endmodule

moore.module @Foo();
endmodule

moore.module @Foo_0();
endmodule

moore.module @Top();
  moore.instance "foo1" @Foo;
  moore.instance "foo2" @Foo_0;
endmodule

moore.module @Foo();
endmodule

moore.module @Foo_0();
endmodule

So when we create a new module, do we need to estimate whether the module exists? If it lands, the fourth step will be removed, right? And should we need an option to control which moore IR will be produced? @mingzheTerapines consider we need, but I don't think so.

[fabianschuiki]

@hailongSun2000 Yes exactly, that would be ideal. Most of the hardware EDA tool flow is built around instances and replication, and not having a design unrolled into a unique module per instance. It would be great if we could preserve this in the Moore dialect 🙂!

A lot of simulators and synthesizers rely on being able to compile/synthesize a module once and reuse the result for different instamces (unless some uniquification is done as an optimization for better results).

If possible, we shouldn't even create IR for duplicate modules. In ImportVerilog, we should keep a hash map of all modules that we have already lowered, and when we see an instance, we should first check if that module has already been lowered, and reuse that. We'll probably want to have a wrapper type around modules as a key into that hash map: the wrapper would hash the entire module if asked to do so, and would structurally compare two modules when asked for equality. Similar to that diacussion with Mikr Popolski.

[MikePopoloski]

Note that it's extremely tricky to get this right in 100% of cases, which is why I still haven't done it in slang (though it's still on my TODO list). Hashing based on parameters and interface ports will get you most of the way there, but you still have to account for hierarchical references, both downward into the instance as well as upward from any child instances that extend up out of the instance being evaluated. You also need to handle defparams and bind directives pointing into the instance from anywhere else in the design, as well as any rules in any active SystemVerilog Configurations that might swap out an instance body anywhere underneath you.

[hailongSun2000]

I don't know how to understand downward... and upward.... So, I asked perplexity for an example:

module Top;
    logic [7:0] top_data_1 = 8'hAA;
    logic [7:0] top_data_2 = 8'hBB;

    Mid mid_inst_1 (.select(1'b0));
    Mid mid_inst_2 (.select(1'b1));
endmodule

module Mid(input logic select);
    Bottom bottom_inst(.select(select));
endmodule

module Bottom(input logic select);
    logic [7:0] bottom_data;

    initial begin
        bottom_data = select ? Top.top_data_2 : Top.top_data_1;
        $display("Bottom data: %h", bottom_data);
    end
endmodule

Maybe this can help you @mingzheTerapines to handle duplicated modules. We must be careful when estimating whether two instance modules are equivalent.

[mingzheTerapines]

@MikePopoloski Would you have a look at this PR #7207
I found slang could emit this error "error: duplicate definition of xxx" if there are two same name modules under one same module.
So I chose to compare parent symbol and name to define they are the duplicate modules. This can dedup same-reference module but not all equipllance modules.
What's more, I will also add compasion with parameter values.

[MikePopoloski]

You shouldn't really need the name of the module; you can check the pointer value of the DefinitionSymbol and unique-ify based on that. There is one and only one DefinitionSymbol per definition of a module/interface/program in the original source. Otherwise you have to worry about not just nested modules but also cases like having two modules with the same name defined in different source libraries.

[mingzheTerapines]

@MikePopoloski Thanks, I have just checked json file, that is very helpful.
What's more, I still need to check parameters for defining duplication.

[fabianschuiki]

Thanks a lot for chiming in here @MikePopoloski, really appreciated 😃 Slang is a fantastic piece of work btw 🥳