llvm / circt

Circuit IR Compilers and Tools
https://circt.org
Other
1.64k stars 285 forks source link

[Moore][Dedup] Dedup module op #7190

Open mingzheTerapines opened 3 months ago

mingzheTerapines commented 3 months ago

https://github.com/llvm/circt/pull/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 commented 3 months ago

@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 commented 3 months ago

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: https://github.com/MikePopoloski/slang/discussions/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 😄

hailongSun2000 commented 3 months ago

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
  2. 
    moore.module Top();
    endmodule

moore.module Foo(); endmodule

3.

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

moore.module Foo(); endmodule

4.

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

moore.module @Foo(); endmodule

moore.module @Foo_0(); endmodule

5.

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@MikePopoloski Would you have a look at this PR https://github.com/llvm/circt/pull/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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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