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

[HW] Fix instance ops in hw-flatten-io pass #5537

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

mortbopet
Copy link
Contributor

No description provided.

Copy link
Contributor

@teqdruid teqdruid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does TypeConverter not work for CallLike ops to do this? It seems like if you're updating function args, you'd need to update the call args as well, and I would imagine that's a big use case for type converter.

If not, this seems like a reasonably common enough problem to warrant a helper lib in MLIR...

lib/Dialect/HW/Transforms/FlattenIO.cpp Outdated Show resolved Hide resolved
lib/Dialect/HW/Transforms/FlattenIO.cpp Show resolved Hide resolved
return success();
}

mutable DenseSet<hw::InstanceOp> *convertedOps;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think mutable is necessary here since the pointer itself isn't modified.

@mortbopet
Copy link
Contributor Author

Does TypeConverter not work for CallLike ops to do this? It seems like if you're updating function args, you'd need to update the call args as well, and I would imagine that's a big use case for type converter.

Are you talking about this, and moving that into the type converter source/target materialization code?

I actually suspect that this pass could be rewritten to use our TypeConversionPattern fairly efficiently... WDYT? Although for now i just want a fix in... hence merging.

@mortbopet mortbopet merged commit 430f215 into main Jul 7, 2023
5 checks passed
@teqdruid
Copy link
Contributor

I actually suspect that this pass could be rewritten to use our TypeConversionPattern fairly efficiently... WDYT? Although for now i just want a fix in... hence merging.

I had forgotten about that, but yeah.

calebmkim pushed a commit to andrewb1999/circt that referenced this pull request Jul 12, 2023
@darthscsi darthscsi deleted the dev/mpetersen/flatten_io_fix_instance branch June 4, 2024 14:48
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

Successfully merging this pull request may close these issues.

2 participants