llvm / circt

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

[Moore] Add AssignedVarOp and canonicalization for VariableOp. #7251

Closed hailongSun2000 closed 2 months ago

hailongSun2000 commented 3 months ago

The canonicalization for VariableOp is aimed at merging the "easy use" of the variable and its value(continuous assignment) into a new op called assigned_variable. Don't handle blocking_assign and nonBlocking_assign due to the dominance. The "easy use" is assigning a value to a variable with the whole bit-width, in other words, exclude the bits slice, concatenation operator, etc.

hailongSun2000 commented 3 months ago

Appendix:

/home/phoenix/work/HDLBits/test01.sv:4:10: error: Unsupported drive the same wire 'l' two times now
  assign l = 1'b0;
         ^
module {
  moore.module @Foo() {
    %l = moore.net wire : <l1>
    %l_0 = moore.assigned_variable name "l" %1 : <l1>
    %0 = moore.constant true : i1
    %1 = moore.conversion %0 : !moore.i1 -> !moore.l1
    moore.assign %l, %1 : l1
    %2 = moore.constant false : i1
    %3 = moore.conversion %2 : !moore.i1 -> !moore.l1
    moore.assign %l, %3 : l1
    moore.output
  }
}
hailongSun2000 commented 3 months ago

I'll separate the changing of SVModule into a different PR.

fabianschuiki commented 3 months ago

I'm wondering if this could work as a canonicalization pattern defined on VariableOp :thinking:. The pattern could theoretically go through all users of the variable op and if there is a single continuous assignment, the assigned value could be merged into a new AssignedVariableOp. But I'm not sure how idiomatic and scalable it is to scan an entire user list in a canonicalization... @uenoku may have an opinion on that :smirk:

uenoku commented 3 months ago

It makes sense to implement it as canonicalization. We do the same thing for SV so compile time should be OK. However in that case please make sure lowering pass can lower them even without canonicalization.

hailongSun2000 commented 3 months ago

Hey, @fabianschuiki @uenoku! I add the canonicalization pattern for moore.variable. But it seems can not keep the original variable, if not call replaceOp() /replaceOpWithNewOp(), the canonicalize() will not take effect.

uenoku commented 3 months ago

Can we remove mergeAssignments pass if we implemented it as a canonicalization?

hailongSun2000 commented 2 months ago

I have removed the MergeAssignments pass.

hailongSun2000 commented 2 months ago

Thanks for the code review :smiley: .

fabianschuiki commented 2 months ago

Very cool! 🥳