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

eDSL: no cells list when defining new component #2038

Merged
merged 12 commits into from
May 15, 2024

Conversation

anshumanmohan
Copy link
Contributor

@anshumanmohan anshumanmohan commented May 9, 2024

As #2034 shows, it is possible to create a free-floating list of cells prior to the creation of a component, and to then associate those cells with a component when creating the component.

This is a little dicey and inelegant. This PR removes that power. You must now create a component, and then construct and insert cells specifically into that component.

This change has no victims in the existing codebase, since NTT (the only user of this feature) has stopped using it as of #2034.

If someone really wanted to, say, insert some "basic" cells into a variety of components and then add more stuff per component, like so:

def manifest_basic_cells_in_a_vacuum() -> list[Cell]:
  // elided
comp1 = prog.component("comp1", cells=manifest_basic_cells_in_a_vacuum())
comp2 = prog.component("comp2", cells=manifest_basic_cells_in_a_vacuum())
// add more cells to comp1 and comp2 as needed

They can still do that:

def add_basic_cells_to_comp(comp) -> ():
  // elided
comp1 = prog.component("comp1")
add_basic_cells_to_comp(comp1)
comp2 = prog.component("comp2")
add_basic_cells_to_comp(comp2)
// add more cells to comp1 and comp2 as needed

@rachitnigam
Copy link
Contributor

Awesome! This also brings it in line with the interface of the Rust-based builder which only allows construction of cells on a particular component.

@anshumanmohan
Copy link
Contributor Author

Super. I'll just leave this PR open in case @sampsyo, @EclecticGriffin, or @calebmkim know of a good reason to keep this feature around!

Base automatically changed from ntt-modern-edsl to main May 13, 2024 14:56
@EclecticGriffin
Copy link
Collaborator

No objections here!

@anshumanmohan anshumanmohan enabled auto-merge (squash) May 15, 2024 02:07
@anshumanmohan anshumanmohan merged commit 1be68ce into main May 15, 2024
18 checks passed
@anshumanmohan anshumanmohan deleted the no-cells-def-component branch May 15, 2024 02:21
jiahanxie353 pushed a commit to jiahanxie353/calyx that referenced this pull request May 29, 2024
* Neaten mul_group

* Neaten another helper

* A little more neatening

* Less use of Cell

* Reduce manual imports

* Catch up to main

* Stray doctring issue

* No cells when defining component

* More thoroughly remove the power to init a component with list of cells
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.

3 participants