Assign integer expression to struct type scalar

[jcasas00]

def test_struct_scalar_twice():
    hcl.init()

    dt8='int8'
    dt32=hcl.Int(32)
    def kernel():
        stype = hcl.Struct({"x": dt8, "y": dt8})
        xy = hcl.scalar(0x12, "foo", dtype=stype).v

        # this also generates the same error test_struct_sideeffect
        stype2 = hcl.Struct({"a": dt8, "b": dt8})
        ab = hcl.scalar(xy.x, "ab", dtype=stype2).v     # <-- replace xy.x with a constant and it builds                                            

        r = hcl.compute((2,), lambda i: 0, dtype=dt32)
        r[0] = xy.y
        r[1] = ab.a
        return r
    s = hcl.create_schedule([], kernel)
    hcl_res = hcl.asarray(np.zeros((2,), dtype=np.int32), dtype=dt32)
    f = hcl.build(s)

The code above generates an LLVM segfault:

Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var LLVM_SYMBOLIZER_PATH to point to it):
/home/jcasas/dprive/heterocl_mlir/hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/_mlir_libs/libHCLMLIRAggregateCAPI.so.15(+0x11b2584)[0x149f604d4584]
/home/jcasas/dprive/heterocl_mlir/hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/_mlir_libs/libHCLMLIRAggregateCAPI.so.15(+0x11afce4)[0x149f604d1ce4]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x143c0)[0x149f6c78e3c0]
/home/jcasas/dprive/heterocl_mlir/hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/_mlir_libs/libHCLMLIRAggregateCAPI.so.15(+0xf0b776)[0x149f6022d776]
/home/jcasas/dprive/heterocl_mlir/hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/_mlir_libs/libHCLMLIRAggregateCAPI.so.15(+0xf53ae0)[0x149f60275ae0]
...

Not sure but this may be related to the comment about 'single expr' initialization not supported yet (#133 (comment))

[zzzDavid]

Yes it's related to #133 , general expr to initialize scalar with struct type is WIP

[zzzDavid]

Segmentation fault is caused by an unrealized_conversion_cast operation from i8 to !hcl.struct<i8, i8>.

1. We should support initializing struct with general expression. However, I think we should restrict the bitwidth of the expression to be the same as the struct type. Otherwise there's ambiguity of how the struct fields should be initialized.
2. We should throw error instead of warning for unrealized_conversion_cast, since it's not supported in execution engine anyway.

[zzzDavid]

@jcasas00 What is the expected behavior when we cast i8 to !hcl.struct<i8, i8>? Should we split i8 to i4 i4 first and extend them to i8 or extend i8 to i16 first then split?

[zhangzhiru]

What is the expected behavior when we cast i8 to !hcl.struct<i8, i8>

I suggest we generate an error (not warning) instead.

[jcasas00]

I think the most reasonable behavior would be the latter one -- extend the source data to the target type's size and then assign.

[jcasas00]

Rethinking this ... as long as an explicit cast works (i.e., hcl.scalar(hcl.cast("int16", ), ...)), then generating an error is fine. This way the cast is forced to be explicit.

[zzzDavid]

This issue should be fixed by ddcb3d4.

I agree that the cast should be explicit. The current integer to struct checks two things:

• The integer bitwidth is the same as the bitwidth sum from the struct type
• All fields of struct are integers, i.e. no automatic bitcast.

I've tested it with this test case:

def test_struct_scalar_twice():
    hcl.init()

    dt4='int4'
    dt8='int8'
    dt32=hcl.Int(32)
    def kernel():
        stype = hcl.Struct({"x": dt8, "y": dt8})
        xy = hcl.scalar(0x12, "foo", dtype=stype).v

        stype2 = hcl.Struct({"a": dt4, "b": dt4})
        ab = hcl.scalar(xy.x, "ab", dtype=stype2).v                               

        r = hcl.compute((2,), lambda i: 0, dtype=dt32)
        r[0] = xy.y
        r[1] = ab.a
        return r
    s = hcl.create_schedule([], kernel)
    hcl_res = hcl.asarray(np.zeros((2,), dtype=np.int32), dtype=dt32)
    f = hcl.build(s)
    f(hcl_res)
    assert hcl_res.asnumpy()[0] == 0
    assert hcl_res.asnumpy()[1] == 2

More test cases will be added.

[jcasas00]

Regarding "All fields of struct are integers, i.e. no automatic bitcast." , I think this may be too restrictive.
I'm interpreting this as saying we can't assign to a struct that has a field that is also a struct. For example:

    t1 = hcl.Struct({"x": hcl.UInt(8), "y": hcl.UInt(8)})
    t2 = hcl.Struct({"z": hcl.UInt(8), "t": t1})

I think as long as the value being assigned has the right bit width (e.g., 24 for a scalar assignment to type t2), the system should be able to assign the fields even if they are non-integers since the bit widths should match.

That said -- I recall we discussed with Sean before about supporting structs in structs but I don't think it was fully implemented. It would be a good enhancement to have to make data abstractions simpler to manage.

[jcasas00]

Misclicked ... didn't mean to close

[zzzDavid]

I agree, I think we should support nested structs. What I meant by "no automatic bitcast" is that we don't do integer slice -> float/fixed bitcast.

[jcasas00]

This change seems to have caused a segfault in some other areas that used to work ...

If the following code is added:

with hcl.if_(xy.y > 0):    # add this "if" condition to the test case above ...
     r[0] = xy.y
     r[1] = ab.a

I see the following error:

  warnings.warn(self.message, category=self.category)
error: 'affine.load' op operation destroyed but still has uses
LLVM ERROR: operation destroyed but still has uses

[zzzDavid]

The AST visitor removed the condition operation while it still has uses:
https://github.com/cornell-zhang/hcl-dialect-prototype/blob/ddcb3d42ba4e3dcb0153245d55ba86b2913c1981/include/hcl/Bindings/Python/hcl/build_ir.py#L2685-L2687

Actually I don't think the new changes broke it, this is the same issue as #141. We need to revamp the logical operations.

[zzzDavid]

This issue is caused by struct op visitors. When their operands are removed but struct ops stayed, MLIR raises an "operation destroyed but still has uses" error. I have added remove and profile mode for struct op visitor with this commit: 7906b26.

[jcasas00]

Thanks. Looks like this one works now.

[jcasas00]

Removing the urgent tag on this as the primary issue has been resolved.