Using hcl.scalar with struct dtype fails

[jcasas00]

The following code passes with the main branch:

def test_struct_scalar():
    hcl.init()
    def kernel():
        stype = hcl.Struct({"x": hcl.UInt(8), "y": hcl.UInt(8)})
        xy = hcl.scalar(0x1234, "foo", dtype=stype).v

        z1 = hcl.compute((2,), lambda i: 0, dtype=hcl.UInt(32))
        z1[0] = xy.y
        z1[1] = xy.x
        return z1

    s = hcl.create_schedule([], kernel)
    hcl_res = hcl.asarray(np.zeros((2,), dtype=np.uint32), dtype=hcl.UInt(32))
    f = hcl.build(s)
    f(hcl_res)
    np_res = hcl_res.asnumpy()
    golden = np.zeros((2,), dtype=np.uint32)
    golden[0] = 0x12
    golden[1] = 0x34
    assert np.array_equal(golden, np_res)
    print("test_struct_scalar passed")

test_struct_scalar()

With hcl-mlir, I get the following error:
File "/home/jcasas/dprive/heterocl_mlir/hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/build_ir.py", line 901, in build
raise DTypeError(
hcl_mlir.exceptions.DTypeError: [Data Type] Type error: unrecognized type: !hcl.struct<ui8, ui8>

[zzzDavid]

There are three things to be supported/fixed for such cases:

• For now struct construction only support passing in a tuple. We'll support initialize a struct from a single value (expression).
• Passing tuple to hcl.scalar
• Somehow struct initialization doesn't support UInt, this issue will be fixed

[zzzDavid]

The error hcl_mlir.exceptions.DTypeError: [Data Type] Type error: unrecognized type: !hcl.struct<ui8, ui8> is in ConstantOp. We should also support initialize struct from constant

[zzzDavid]

The UInt issue is fixed by 025e533.

When constructing the AST, the ExprOp nodes retains the sign information, i.e. an unsigned integer type is represented as ui<b>. However, when ExprOp.build() is called to generate the IR, all signed/unsigned integer should be generated as signless integer.

[zzzDavid]

Up to this commit 0f1802c, passing tuple and integer to hcl.scalar for a struct scalar is supported.

This particular test has passed and is added to the test suite here:
https://github.com/cornell-zhang/heterocl/blob/601cae16769fad2f5340e4aa03d8e3ef0753a251/tests/mlir/test_dtype.py#L567-L587

However, hcl.scalar doesn't have complete support for creating struct from a single expr yet.

[jcasas00]

The following kernel code also generates an error:

    def kernel():
        stype = hcl.Struct({"x": hcl.UInt(8), "y": hcl.UInt(8)})
        xy = hcl.scalar(0x12, "foo", dtype=stype).v
        r = hcl.compute((2,), lambda i: 0, dtype=hcl.UInt(32))
        // removed r[0] = xy.x ...
        return r

In this case, xy is NOT used but somehow the hcl.scalar w/ struct dtype is causing the following error:

File "python/heterocl/build_module.py", line 360, in build_llvm
execution_engine = ExecutionEngine(module, opt_level=0)
RuntimeError: Failure while creating the ExecutionEngine.

If hcl.scalar used a non-struct dtype (e.g., uint32), no runtime error is generated.

[zzzDavid]

This issue is caused by how structs are lowered in the Conversion pass. The current struct lowering looks at the where the struct is used, and then get the field value from the struct construct op. So when there's no usage, the struct doesn't gets lowered (i.e. folded). I'll add DCE to resolve this issue.

[zzzDavid]

This is more interesting than I thought, a simple DCE won't suffice:

module {
  func.func @top() -> memref<2xi32> attributes {itypes = "", otypes = "u"} {
    %0 = memref.alloc() {name = "foo"} : memref<1x!hcl.struct<i8, i8>>
    %c18_i8 = arith.constant {unsigned} 18 : i8
    %c0_i8 = arith.constant {unsigned} 0 : i8
    %1 = hcl.struct_construct(%c18_i8, %c0_i8) : i8, i8 -> <i8, i8>
    affine.store %1, %0[0] {to = "foo"} : memref<1x!hcl.struct<i8, i8>>
    %2 = memref.alloc() {name = "compute_0", unsigned} : memref<2xi32>
    affine.for %arg0 = 0 to 2 {
      %c0_i32 = arith.constant {unsigned} 0 : i32
      affine.store %c0_i32, %2[%arg0] {to = "compute_0"} : memref<2xi32>
    } {loop_name = "i", op_name = "compute_0"}
    return %2 : memref<2xi32>
  }
}

The struct_construct op isn't erased because it has one use: store to the memref. In this case, %0 was allocated but not used, but its initialization uses the struct construct, that's why DCE doesn't erase it.

[zzzDavid]

I think we should add a pass to remove all memrefs allocated, initialized, but never used (loaded from).

[jcasas00]

I agree that allocated/initialized but unused memrefs should be removed -- shouldn't this be part of DCE already?
That said, I think there's something else going on since even if the struct is unused, it should not cause runtime errors (i.e., unused memrefs is sensitizing the real issue).

[zzzDavid]

I think MLIR should have DCE passes available for removing unused memrefs. The runtime error of RuntimeError: Failure while creating the ExecutionEngine is actually saying there's struct operations in the IR, but the execution engine is expecting LLVM dialect ops only. I will add checking at the end of LowerCompositeType pass, if there's still struct type or ops, users will get an error with more useful information

[zzzDavid]

Added test case for unused struct tensor: mlir/test_dsl_basic.py::test_unused_struct_tensor with this commit: cornell-zhang/heterocl@bf49d66

[zzzDavid]

Closing the thread as the issue has been resolved