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

[FIRRTL][ExpandWhens] Support flow checking for local and remote objects #6212

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

rwy7
Copy link
Contributor

@rwy7 rwy7 commented Sep 27, 2023

Objects can be classified as either local or remote. A local object is an actual firrtl.object declaration in the current scope. A remote object would be a class-typed port (of either the current module/class, or an instance/object).

For local objects, we are obligated to initialize their input ports. Sink-flow remote objects must be initialized by assigning to the entire thing, at the root.

This PR does a few things:

  1. implement Field IDs for ClassType
  2. implement FieldRef and field name utilities for objects
  3. make expand-whens an interface pass that runs on FModuleLike operations, so that we can run it on class bodies in addition to modules.
  4. In expand-whens, add support for flow checking of objects

@rwy7 rwy7 force-pushed the classes-exand-whens branch 2 times, most recently from 07b8a48 to 7934d8d Compare September 27, 2023 21:21
@rwy7 rwy7 marked this pull request as ready for review September 27, 2023 21:23
@rwy7 rwy7 added the FIRRTL Involving the `firrtl` dialect label Sep 27, 2023
Copy link
Contributor

@dtzSiFive dtzSiFive left a comment

Choose a reason for hiding this comment

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

LGTM, added some nits and comments. Nice work!

lib/Dialect/FIRRTL/Transforms/ExpandWhens.cpp Outdated Show resolved Hide resolved
lib/Dialect/FIRRTL/Transforms/ExpandWhens.cpp Outdated Show resolved Hide resolved
lib/Dialect/FIRRTL/Transforms/ExpandWhens.cpp Show resolved Hide resolved
lib/Dialect/FIRRTL/Transforms/ExpandWhens.cpp Outdated Show resolved Hide resolved
test/Dialect/FIRRTL/expand-whens.mlir Outdated Show resolved Hide resolved
test/Dialect/FIRRTL/expand-whens.mlir Outdated Show resolved Hide resolved
@@ -494,7 +494,8 @@ FieldRef circt::firrtl::getDeltaRef(Value value, bool lookThroughCasts) {
return FieldRef();
return FieldRef(op.getInput(), 0);
})
.Case<SubfieldOp, OpenSubfieldOp, SubindexOp, OpenSubindexOp, RefSubOp>(
.Case<SubfieldOp, OpenSubfieldOp, SubindexOp, OpenSubindexOp, RefSubOp,
ObjectSubfieldOp>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Woohoo! 👍

Gets us slightly better error messages for flow checking as a result.
@rwy7 rwy7 force-pushed the classes-exand-whens branch 3 times, most recently from 026f6e5 to 054dd41 Compare September 28, 2023 15:22
@rwy7 rwy7 merged commit 8f66727 into llvm:main Oct 3, 2023
5 checks passed
@rwy7 rwy7 deleted the classes-exand-whens branch October 3, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIRRTL Involving the `firrtl` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants