Incorrect memref type generated for struct
Closed this issue · 15 comments
def test_struct_uint():
hcl.init()
rshape = (1,)
def kernel():
stype = hcl.Struct({"x": 'uint8', "y": 'uint8'})
ival = hcl.cast('uint16', 0)
d = hcl.scalar(ival, "d", dtype=stype).v
r = hcl.compute(rshape, lambda _:0, dtype=hcl.UInt(32))
r[0] = d.x
return r
#
s = hcl.create_schedule([], kernel)
print(hcl.lower(s))
hcl_res = hcl.asarray(np.zeros(rshape, dtype=np.uint32), dtype=hcl.UInt(32))
f = hcl.build(s)
f(hcl_res)
np_res = hcl_res.asnumpy()
golden = np.zeros(rshape, dtype=np.uint32)
golden[0] = 0
assert np.array_equal(golden, np_res)
generates the following error:
"func.func"() ({
%0 = "arith.constant"() {value = 0 : index} : () -> index
%1 = "arith.constant"() {value = 0 : i32} : () -> i32
%2 = "arith.trunci"(%1) {unsigned} : (i32) -> i16
%3 = "memref.alloc"() {name = "d", operand_segment_sizes = dense<0> : vector<2xi32>} : () -> memref<1x!hcl.struct<i8, i8>>
%4 = "arith.constant"() {value = 0 : index} : () -> index
%5 = "hcl.int_to_struct"(%2) : (i16) -> !hcl.struct<ui8, ui8>
"affine.store"(%5, %3) {map = #map0, to = "d"} : (!hcl.struct<ui8, ui8>, memref<1x!hcl.struct<i8, i8>>) -> ()
%6 = "affine.load"(%3) {from = "d", map = #map0} : (memref<1x!hcl.struct<i8, i8>>) -> !hcl.struct<i8, i8>
...
error: 'affine.store' op value to store must have the same type as memref element type
The type of %3 is <i8,i8> even if stype has unsigned ints? Seems like a bug ...
The hcl.cast result in %5 has the expected type of <ui8,ui8>.
So the error is because it is trying to store a <ui8,ui8> to a <i8,i8>.
When we build the IR we have to convert all signed/unsigned integer types to signless integer types. This restriction is imposed by the MLIR verifier (specifically the Arithmetic dialect's operation verifier). For now we are missing a converter for struct type that recursively convert all signed/unsigned integer fields to signless. I will add that to fix this issue.
I wanted to mark this issue as "urgent" but looks like I'm not able to (or don't know how to).
Hi Jeremy, I upgraded your permission, I think you should be able to manage issue tags now. I'll fix this asap
Looks like I should be able to update the labels now. Thanks!
After updating (heterocl and hcl-dialect-prototype), I'm now getting:
File "python/heterocl/operation.py", line 85, in scalar
hcl_mlir.StoreOp(init, ret_tensor.op, [index])
File "hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/build_ir.py", line 2029, in __init__
val = CastOp(val, to_tensor.dtype)
File "hcl-dialect-prototype/build/tools/hcl/python_packages/hcl_core/hcl_mlir/build_ir.py", line 1701, in __init__
raise DTypeError(
hcl_mlir.exceptions.DTypeError: [Data Type] Unrealized conversion cast: ui16 -> !hcl.struct<ui8, ui8>
Ah we are discussing changing an API to restrict the behavior of "using struct as an integer" to avoid opening a can of worm with "casting between integer and struct". The API change removed integer to struct as a cast operation, so you are seeing this error.
The API change is still being finalized, could you try this commit point and see if works: 4ccd659
Thanks for confirming. I'll update in this thread once we finalize the API design
I'm seeing the following errors that seem to be related to this issue (still using 4ccd659) :
Error: [Pass][LowerCompositeType] structConstructOp is not legal: %209 = hcl.struct_construct(%207, %208) : i7, i2 -> <i7, i2>
Error: [Pass][LowerCompositeType] structConstructOp is not legal: %213 = hcl.struct_construct(%211, %212) : i7, i2 -> <i7, i2>
Error: [Pass][LowerCompositeType] structConstructOp is not legal: %212 = hcl.struct_construct(%210, %211) : i7, i2 -> <i7, i2>
Error: [Pass][LowerCompositeType] structConstructOp is not legal: %216 = hcl.struct_construct(%214, %215) : i7, i2 -> <i7, i2>
...
loc("-":2:3): error: Lowering composite type failed
loc("-":2229:24): error: failed to legalize operation 'memref.alloc'
The IR after create_schedule / before build is attached.
This is caused by struct_construct ops generated after int_to_struct ops are lowered. I have fixed on my end and passed soc_top test, will push after adding tests.
Hi @jcasas00, I fixed this issue on the latest commits. I was able to lower soc_top to LLVM dialect.
Could you pull from both repo and test if it works?
Yes, this issue seems to be resolved now. Thanks.
Removing urgent tag as the primary issue has been resolved.
A note on signed/unsigned/signless type in the IR. The revamped frontend cornell-zhang/heterocl#473 uses HeteroCL's type for type inference and type judgment. That means we can get the data type of any expression by expr.dtype, which returns a hcl.Type object, such as hcl.UInt(8) or hcl.Int(32). Therefore, when we build the IR we use signless integer, since we no longer need signed/unsigned MLIR integer type for type judgment, and the above error will not occur since we only build signless integer MLIR type in the IR.
Closing the thread as the issue has been resolved.
Discussion about struct expr's .as_int() API will move to #147