Skip to content

Conversation

@meheff
Copy link
Collaborator

@meheff meheff commented Oct 16, 2025

This significantly reduces the size of the verilog for these operations. Also reduce unnecessary duplication of expressions in some ops, and inline bits typed literals.

Copy link
Contributor

@grebe grebe left a comment

Choose a reason for hiding this comment

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

This is nice! Thanks for doing this

@meheff meheff force-pushed the meheff/10-16-2025-array-update-generate branch from ed1171b to 1423b5a Compare October 20, 2025 06:01
@meheff
Copy link
Collaborator Author

meheff commented Oct 20, 2025

The failure //xls/jit:jit_channel_queue_test is unrelated and seems to be broken at head at least for github CI.

@meheff meheff requested a review from grebe October 20, 2025 19:47
@allight
Copy link
Contributor

allight commented Oct 23, 2025

Would you mind rebasing and re-running rebuild-goldens? That script was missing a bunch of tests it should have included but it has been updated now.

This significantly reduces the size of the verilog for these operations. Also reduce unnecessary duplication of expressions in some ops.
@meheff meheff force-pushed the meheff/10-16-2025-array-update-generate branch from 1423b5a to 5d05563 Compare October 29, 2025 03:43
@meheff
Copy link
Collaborator Author

meheff commented Oct 29, 2025

(I missed your earlier comment). Updated and fixed tests. Also fixed the golden file script which was broken in OSS.

Copy link
Contributor

@ericastor ericastor left a comment

Choose a reason for hiding this comment

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

LGTM, but I'll let @grebe weigh in before this lands.

@meheff
Copy link
Collaborator Author

meheff commented Oct 29, 2025

Add "Reviewing Internally" label? Thanks

@copybara-service copybara-service bot merged commit eaba202 into google:main Oct 31, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants