[MooreToCore] VariableOp lowered failed

[mingzheTerapines]

Dear @maerhart @fabianschuiki , When lowering SV to Hw Dialect, there is a stack dump. Driver: circt-verilog %s

module top();
  typedef struct {
    int a;
    int b;
  } ms_t;

  ms_t ms;

  initial begin
    ms = '{ 0, 1};

    ms = '{ default:1, int:1};

    ms = '{ int:0, int:1};
  end

endmodule

It can be converted to moore Dialect like this Driver: circt-verilog --ir-moore %s

module {
  moore.module @top() {
    %0 = moore.constant 1 : i32
    %1 = moore.constant 0 : i32
    %ms = moore.variable : <ustruct<{a: i32, b: i32}>>
    moore.procedure initial {
      %2 = moore.struct_create %1, %0 : !moore.i32, !moore.i32 -> ustruct<{a: i32, b: i32}>
      moore.blocking_assign %ms, %2 : ustruct<{a: i32, b: i32}>
      %3 = moore.struct_create %0, %0 : !moore.i32, !moore.i32 -> ustruct<{a: i32, b: i32}>
      moore.blocking_assign %ms, %3 : ustruct<{a: i32, b: i32}>
      moore.blocking_assign %ms, %3 : ustruct<{a: i32, b: i32}>
      moore.return
    }
    moore.output
  }
}

But it got stack dump when casting hw::InOutType. Maybe structType should be converted somehow. This is part of error codes.

#19 0x00005f9660f42af4 (anonymous namespace)::OperationLegalizer::legalizeWithPattern(mlir::Operation*, mlir::ConversionPatternRewriter&) /home/pluto/Documents/circt/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:1958:21
#20 0x00005f9660f3b100 (anonymous namespace)::OperationLegalizer::legalize(mlir::Operation*, mlir::ConversionPatternRewriter&) /home/pluto/Documents/circt/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:1850:17
#21 0x00005f9660f3aa73 mlir::OperationConverter::convert(mlir::ConversionPatternRewriter&, mlir::Operation*) /home/pluto/Documents/circt/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:2384:26
#22 0x00005f9660f3b41f mlir::OperationConverter::convertOperations(llvm::ArrayRef<mlir::Operation*>) /home/pluto/Documents/circt/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:2436:16
#23 0x00005f9660f3fdfc mlir::applyFullConversion(llvm::ArrayRef<mlir::Operation*>, mlir::ConversionTarget const&, mlir::FrozenRewritePatternSet const&, mlir::ConversionConfig) /home/pluto/Documents/circt/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:3447:22
#24 0x00005f9660f3fe9d mlir::applyFullConversion(mlir::Operation*, mlir::ConversionTarget const&, mlir::FrozenRewritePatternSet const&, mlir::ConversionConfig) /home/pluto/Documents/circt/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:3453:10
#25 0x00005f965faed60d (anonymous namespace)::MooreToCorePass::runOnOperation() /home/pluto/Documents/circt/circt/lib/Conversion/MooreToCore/MooreToCore.cpp:1398:14

[mingzheTerapines]

Maybe we should have code like this:

Type elementType = cast<hw::InOutType>(resultType).getElementType();
->
if (auto elementType = cast<hw::InOutType>(resultType).getElementType())

[maerhart]

I think the reason this fails is because there is no type conversion registered for ustruct in MooreToCore.cpp (and also various other aggregate types defined in MooreTypes.td). However, there's a type converter registered that converts any type to itself if no other converter could convert it. So that ustruct just stays as is and thus the cast fails.

Which line are you exactly referring to in the last comment?

[mingzheTerapines]

@maerhart Sorry for letting you reply so late in the night. Here is the line I am referring. https://github.com/llvm/circt/blob/fb052d901ad2c4aeef885887a5868e1eeedd49e0/lib/Conversion/MooreToCore/MooreToCore.cpp#L468 Hope you sleep well then.

[maerhart]

That line should be fine. It's verified the the variable op that the result is a ref type which is lowered to an inout.

[mingzheTerapines]

You are right, ustruct type is not supported, which may need some annotation like lines after that, such as

// to_do : support ustruct type

[fabianschuiki]

We could convert unpacked structs to !hw.struct to get started. If I recall correctly, the differences are fairly subtle, and mostly affect data layout and granularity of event tracking in simulation. Maybe mapping to the plain old packed types could be a start?

[mingzheTerapines]

@fabianschuiki It seems can solve this problem.

[rubenbuchatskiy]

Similar issue with UnpackedArrayType. Segmentation fault on the following code because there is no type conversion for uarray:

module m();
  reg [1:0] regs [0:1];

  initial begin
      regs[0] = 0;
  end
endmodule

[felixtensor]

Due to lack operations related to unpacked array in llhd dialect. We could not establish the bridge for the unpackedArrayType at moment. Personally speaking, there are two methods: 1. materialize the input value with unpackedArrayType to an ArrayType one. 2. add new op to it (or maybe enlarge the type definition of array operations in llhd).

[mingzheTerapines]

@cepheus69 Cool! 3 tests on chipsalliance now fixed. What's more, unpacked array is not supported yet.

[fabianschuiki]

Very nice!