xdslproject / xdsl

A Python Compiler Design Toolkit
Other
275 stars 72 forks source link

API: `builtin.DenseArrayBase.create_dense_int_or_index` fails during verification when an `IndexType` is passed as `data_type` #2604

Open watermelonwolverine opened 6 months ago

watermelonwolverine commented 6 months ago

The method name and the type hints for the data_type suggest that I can pass a IndexType as data_type. Doing so causes an exception during verification.

indices = builtin.DenseArrayBase.create_dense_int_or_index(
    builtin.IndexType(),
    [0],
)

throws:

openpme_xdsl/transforms/convert_particles_to_ll_mlir.py:266: in match_and_rewrite
    indices = builtin.DenseArrayBase.create_dense_int_or_index(
.venv/lib/python3.10/site-packages/xdsl/dialects/builtin.py:1077: in create_dense_int_or_index
    return DenseArrayBase([data_type, ArrayAttr(attr_list)])
.venv/lib/python3.10/site-packages/xdsl/ir/core.py:627: in __init__
    super().__init__()
<string>:3: in __init__
    ???
.venv/lib/python3.10/site-packages/xdsl/ir/core.py:451: in __post_init__
    self._verify()
.venv/lib/python3.10/site-packages/xdsl/ir/core.py:666: in _verify
    super()._verify()
.venv/lib/python3.10/site-packages/xdsl/ir/core.py:456: in _verify
    self.verify()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = DenseArrayBase(parameters=(IndexType(parameters=()), ArrayAttr(data=(IntAttr(data=0),))), elt_type=IndexType(parameters=()), data=ArrayAttr(data=(IntAttr(data=0),)))

    def verify(self):
        if isinstance(self.elt_type, IntegerType):
            for d in self.data.data:
                if isinstance(d, FloatData):
                    raise VerifyException(
                        "dense array of integer element type "
                        "should only contain integers"
                    )
        else:
            for d in self.data.data:
                if isinstance(d, IntAttr):
>                   raise VerifyException(
                        "dense array of float element type "
                        "should only contain floats"
                    )
E                   xdsl.utils.exceptions.VerifyException: dense array of float element type should only contain floats
superlopuh commented 6 months ago

Indeed, looks like we're missing a | IndexType in the elt_type definition, would you mind adding it there and adding a test for your use-case? I'd expect that there wouldn't be many other places to adjust for the type. CC @math-fehr

watermelonwolverine commented 6 months ago

Added PR: https://github.com/xdslproject/xdsl/pull/2633