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] FoldUnusedBits: minor cleanup #7914

Merged
merged 2 commits into from
Nov 28, 2024
Merged

Conversation

rwy7
Copy link
Contributor

@rwy7 rwy7 commented Nov 27, 2024

Two changes:

  • Bail early when all bits in the memory are used, we clearly cannot run the cannonicalizer.
  • Use createOrFold when creating new ops. After shrinking the memory, the readers, which are bit select and concat ops, will typically just be selecting the entire memory anyways, so they can be eagerly folded away.

Once the memory has been replaced with the compressed memory, a lot of the
bitselect ops reading from the old memory will be selecting the entire memory.
Use createOrFold to eagerly clean up these "whole range bitselect" ops.
@rwy7 rwy7 requested review from youngar and seldridge and removed request for seldridge November 27, 2024 23:37
@rwy7 rwy7 added the FIRRTL Involving the `firrtl` dialect label Nov 27, 2024
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines -2694 to +2695
if (!bits) {
usedBits.set();
continue;
}
if (!bits)
return failure();
Copy link
Member

Choose a reason for hiding this comment

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

This early exit makes sense to me. No reason to wait until all users have been visited before bailing.

Comment on lines -2831 to +2841
rewriter.replaceOpWithNewOp<BitsPrimOp>(
readOp, readOp.getInput(),
// Create a new bit selection from the compressed memory. The new op may
// be folded if we are selecting the entire compressed memory.
auto newReadValue = rewriter.createOrFold<BitsPrimOp>(
readOp.getLoc(), readOp.getInput(),
readOp.getHi() - readOp.getLo() + it->second, it->second);
rewriter.replaceAllUsesWith(readOp, newReadValue);
rewriter.eraseOp(readOp);
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense to me.

@rwy7 rwy7 merged commit b16f1ad into llvm:main Nov 28, 2024
4 checks passed
@rwy7 rwy7 deleted the fold-unused-bits-cleanup branch November 28, 2024 13:39
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