[Moore] A new pass to delete local temporary variables.

[terapines-osc-circt]

Hey, @fabianschuiki! Please review this at your convenience! Thanks in advance! This PR is about deleting local variables. For example:

int x, y, z;
always_comb begin
  automatic int a;
  a = x + 1;
  y = a;
  a = a + 1;
  z = a;
end

To

int x, y, z;
y = x + 1;
z = (x + 1) + 1;

Anything about code details could be pointed out.

[terapines-osc-circt]

The further IR details: From

module {
  moore.module @Foo {
    %x = moore.variable : !moore.int
    %y = moore.variable : !moore.int
    %z = moore.variable : !moore.int
    moore.procedure always_comb {
      %a = moore.variable : !moore.int
      %0 = moore.constant 1 : !moore.int
      %1 = moore.add %x, %0 : !moore.int
      moore.blocking_assign %a, %1 : !moore.int
      moore.blocking_assign %y, %a : !moore.int
      %2 = moore.constant 1 : !moore.int
      %3 = moore.add %a, %2 : !moore.int
      moore.blocking_assign %a, %3 : !moore.int
      moore.blocking_assign %z, %a : !moore.int
    }
  }
}

---

To

moore.procedure always_comb {
  %0 = moore.constant 1 : !moore.int
  %1 = moore.add %x, %0 : !moore.int
  moore.blocking_assign %y, %1 : !moore.int
  %2 = moore.constant 1 : !moore.int
  %3 = moore.add %1, %2 : !moore.int
  moore.blocking_assign %z, %3 : !moore.int
}

or From

module Foo;
  int x, y, z;
  bit cond;
  always_latch begin
    automatic int a;
    if(!cond) begin
      a = x + 10;
      y = a;
      if(cond) begin
        a = a + 3;
        z = a;
      end
      else begin
        z = y + a;
      end
    end
  end
endmodule

moore.procedure always_latch {
      %a = moore.variable : !moore.int
      %0 = moore.not %cond : !moore.bit
      %1 = moore.conversion %0 : !moore.bit -> i1
      scf.if %1 {
        %2 = moore.constant 10 : !moore.int
        %3 = moore.add %x, %2 : !moore.int
        moore.blocking_assign %a, %3 : !moore.int
        moore.blocking_assign %y, %a : !moore.int
        %4 = moore.conversion %cond : !moore.bit -> i1
        scf.if %4 {
          %5 = moore.constant 3 : !moore.int
          %6 = moore.add %a, %5 : !moore.int
          moore.blocking_assign %a, %6 : !moore.int
          moore.blocking_assign %z, %a : !moore.int
        } else {
          %5 = moore.add %y, %a : !moore.int
          moore.blocking_assign %z, %5 : !moore.int
        }
      }
    }

---

To

y = x + 10;
if(cond){
z = x + 10 + 3;
}
else{
z = x + 10 + x + 10
}

moore.procedure always_latch {
  %0 = moore.not %cond : !moore.bit
  %1 = moore.conversion %0 : !moore.bit -> i1
  scf.if %1 {
    %2 = moore.constant 10 : !moore.int
    %3 = moore.add %x, %2 : !moore.int
    moore.blocking_assign %y, %3 : !moore.int
    %4 = moore.conversion %cond : !moore.bit -> i1
    scf.if %4 {
      %5 = moore.constant 3 : !moore.int
      %6 = moore.add %3, %5 : !moore.int
      moore.blocking_assign %z, %6 : !moore.int
    } else {
      %5 = moore.add %y, %3 : !moore.int
      moore.blocking_assign %z, %5 : !moore.int
    }
  }
}

The above-mentioned are just one step before generating the Core IR.

[terapines-osc-circt]

All Build and Tests are passing on my personal computer. It's so strange!

[terapines-osc-circt]

Great to see this take shape!

It would be nice to have a bunch of tests that make sure all the different code paths through the pass' code work as expected. And that also make sure that things which the pass does not yet support (for example, myStruct.myField[42] = x) either produce an error message or are left in their original state.

Yeah, I'll test more cases to ensure its correction.

[terapines-osc-circt]

Great to see this take shape!

It would be nice to have a bunch of tests that make sure all the different code paths through the pass' code work as expected. And that also make sure that things which the pass does not yet support (for example, myStruct.myField[42] = x) either produce an error message or are left in their original state.

I have an example:

module Foo;
  int x, y;
  always_comb begin
    typedef struct packed signed {int a, b;} int64;
    int64 ii;
    ii.a = x + 1;                  // error: unsupported expression: MemberAccess
    ii.b = x;
    y = ii.a;
  end
endmodule

Or

module Foo;
  int x, y;
  always_comb begin
    typedef struct packed signed {int a,b;} int64;
    int64 ii = '{ x+1, x};       // error: unsupported expression: SimpleAssignmentPattern
    y = ii;
  end
endmodule

However, these error messages are thrown by importVerilog because we don't implement them. So may I handle/verify the local struct int64 after supporting the related struct?

[fabianschuiki]

By the way, @dtzSiFive has mentioned that MLIR has a mem2reg pass: https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Mem2Reg.cpp

You might be able to rely on that pass to do the heavy lifting for getting rid of local variables.

[terapines-osc-circt]

By the way, @dtzSiFive has mentioned that MLIR has a mem2reg pass: https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Mem2Reg.cpp

You might be able to rely on that pass to do the heavy lifting for getting rid of local variables.

I'll try it again. Originally, I attempted to call createMem2Reg, but it didn't work to delete local variables. Maybe I got the call createMem2Reg wrong.

[terapines-osc-circt]

Yeah, Mem2Reg doesn't handle moore IR. It just looks like --parse-only.

[fabianschuiki]

I think the Mem2Reg pass uses op interfaces to find out what certain ops do and how it has to handle them. You may be able to add those interfaces to the relevant moore ops to make Mem2Reg able to work with them 🙂

[cepheus69]

That means we need new interfaces for specified operations 🤔 So MooreOpInterface is coming ?

[fabianschuiki]

I think all you need is to add the PromotableOpInterface, PromotableOpInterface, or other interface that Mem2Reg uses to the list of traits of the corresponding Moore op definition. The InstanceOp already does this for the SymbolUserOpInterface: https://github.com/llvm/circt/blob/main/include/circt/Dialect/Moore/MooreOps.td#L64-L66

As far as I can tell, only the MemRef and LLVMIR dialects in MLIR support mem2reg, or at least have a test for it: https://github.com/llvm/llvm-project/blob/main/mlir/test/Dialect/MemRef/mem2reg.mlir. But you may be able to look at the op definitions in the MemRef dialect to see which of the Mem2Reg op interfaces you have to implement.

[terapines-osc-circt]

Yeah, that's right! We don't create a new OpInterface for moore dialect. At least not for now. I was yesterday referred to LLVMIR which also uses the PromotableOpInterface and other Promotable*OpInterface. However, I met the ld.lld error(undefined symbol). Maybe I forgot something to link it in the CMakeLists. Like the above image, I separately added the MemoryslotInterfaces.h and MemoryslotInterfaces.td to the MooreOp.h and MooreOp.td, and added MLIRMemorySlotInterfaces/MLIRTransforms(just one of them, I forgot 😢 ) into CMakeLists. But I'll solve it tomorrow.