Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aie.dma_bd (DMABDOp) parsing of optional attributes is fragile/buggy #1921

Open
hunhoffe opened this issue Nov 15, 2024 · 0 comments
Open

aie.dma_bd (DMABDOp) parsing of optional attributes is fragile/buggy #1921

hunhoffe opened this issue Nov 15, 2024 · 0 comments

Comments

@hunhoffe
Copy link
Collaborator

hunhoffe commented Nov 15, 2024

As part of #1919, I was trying to move the length calculation into the dma_bd op itself. I realized that the documentation claims that the len is optional, however, for an operation like this:

aie.dma_bd(%arg2 : memref<65536xbf16>, [<size = 1, stride = 0>, <size = 1, stride = 0>, <size = 1, stride = 0>, <size = 65536, stride = 1>])

I would get a parsing error like this:

error: "-":69:49: expected attribute value

The error originated from this line:

if (parser.parseCustomAttributeWithFallback(

Upon further inspection, I realized parsing of the dma_bd op is fragile/buggy due to the way the sequence of optional attributes is handled. Iflen but not offset is given, the len will be parsed as the offset. Similarly, if pad_dimensions is specified but dimensions is not, pad_dimensions will be parsed as dimensions. Other combinations of optional attributes results in error such as above.

I think the best way to fix this is to:

  • Make all optional attributes keyword arguments
  • Put all optional attribute parsing into a while-loop with condition succeeded(parser.parseOptionalComma()), and then have a switch statement to parse each optional value.

While this is not complex to implement, I believe that this will require updating a large amount of tests. As such, I'm going to leave this as an outstanding TODO and continue calculating the length and providing an offset in a python helper function.

@hunhoffe hunhoffe changed the title aie.dma_bd (DMABDOp) parsing of optional values is buggy aie.dma_bd (DMABDOp) parsing of optional values is fragile/buggy Nov 15, 2024
@hunhoffe hunhoffe changed the title aie.dma_bd (DMABDOp) parsing of optional values is fragile/buggy aie.dma_bd (DMABDOp) parsing of optional attributes is fragile/buggy Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant