llvm / circt

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

[Moore] Add LowerConcatRef pass to handle concat_ref. #7216

Closed hailongSun2000 closed 2 months ago

hailongSun2000 commented 3 months ago

Assignments like {a, b} = c are very tricky to handle directly. So we introduce a new Moore-to-Moore pass called LowerConcatRef. Which is used to disassemble the moore.concat_ref. And it runs before MooreToCore and guarantees never to see moore.concat_ref on the LHS of an assignment. For example:

{a, b} = c;
// becomes
a = c[9001:42];
b = c[41:0];
hailongSun2000 commented 3 months ago

Thanks for the code review in advance! And Thanks @fabianschuiki's suggestion! :heart:

hailongSun2000 commented 3 months ago

Hey, @uenoku! I'm having trouble with nested moore.concat_ref. For example,

moore.module @top() {
  %a = moore.variable : <i32>
  %b = moore.variable : <i32>
  %c = moore.variable : <i32>
  moore.procedure initial {
    %0 = moore.concat_ref %a, %a : (!moore.ref<i32>, !moore.ref<i32>) -> <i64>
    %1 = moore.concat_ref %a, %b : (!moore.ref<i32>, !moore.ref<i32>) -> <i64>
    %2 = moore.concat_ref %0, %1 : (!moore.ref<i64>, !moore.ref<i64>) -> <i128>
    %3 = moore.read %c : i32
    %4 = moore.conversion %3 : !moore.i32 -> !moore.i128
    %5 = moore.conversion %4 : !moore.i128 -> !moore.i128
    moore.blocking_assign %2, %5 : i128
  }
  moore.output
}

I attempted to use OpConversionPattern to rewrite moore.concat_ref, I want to rewrite this case to become

// All `moor.concat_ref` are erased.
moore.blokcing_assign %a, %10      // %10 = extract 32 bits[127:96] from %5
moore.blokcing_assign %a, %11      // %11 = extract 32 bits[95:64] from %5 
moore.blokcing_assign %a, %12      // %12 = extract 32 bits[63:32] from %5
moore.blokcing_assign %b, %13      // %13 = extract 32 bits[31:0] from %5

However, I can only rewrite %2 = moore.concat_ref %0, %1... and moore.blocking_assign %2, %5 : i128 to become

%0 = moore.concat_ref %a, %a : (!moore.ref<i32>, !moore.ref<i32>) -> <i64>
%1 = moore.concat_ref %a, %b : (!moore.ref<i32>, !moore.ref<i32>) -> <i64>
moore.blokcing_assign %0, %20      // %0 = extract 64 bits[127:64] from %5
moore.blokcing_assign %1, %21      // %1 = extract 64 bits[63:0] from %5

It's more complicated than I thought. So, does the OpConversionPattern have a way(like a loop) to handle this situation? But for all I know, OpConversionPattern is executed op-by-op.

uenoku commented 3 months ago

I think the pattern should be recursively applied to %0 = moore.concat_ref %a, %a and %1 = moore.concat_ref %a, %b if we let OpConversion Lister know the operation is replaced. Could you push a change so I take a look?

hailongSun2000 commented 3 months ago

I pushed the changed version.

hailongSun2000 commented 3 months ago

I think the pattern should be recursively applied to %0 = moore.concat_ref %a, %a and %1 = moore.concat_ref %a, %b if we let OpConversion Lister know the operation is replaced. Could you push a change so I take a look?

Maybe I need to think about another way to implement the OpConversionPattern. I use the old implementation.

hailongSun2000 commented 3 months ago

Good idea! Concentrate on assignments if its LHS is concatRef. It's my mind that is solidified.

hailongSun2000 commented 3 months ago

Hey, @uenoku! I changed the implementation of ConcatRefLowering by using @fabianschuiki's suggestion. And don't erase the unused defOp that you point out.

hailongSun2000 commented 2 months ago

Don't be sorry. It's an honor for me to do something for CIRCT. Also very appreciative that you can give some nice suggestions. :smile: