Skip to content

Conditonally add external attribute for memory element #10050

Open
Abhilekhgautam wants to merge 2 commits intollvm:mainfrom
Abhilekhgautam:fix-external
Open

Conditonally add external attribute for memory element #10050
Abhilekhgautam wants to merge 2 commits intollvm:mainfrom
Abhilekhgautam:fix-external

Conversation

@Abhilekhgautam
Copy link
Copy Markdown

Fixes #10036

There are basically two changes:

  1. Sets external : true in top level component and external : false for non top level component.
  2. Doesn't emit external(0) when external: false is set in the emitter

@cgyurgyik
Copy link
Copy Markdown
Member

I'm less familiar with the semantics of external memory. Perhaps @rachitnigam has some input here.

@rachitnigam
Copy link
Copy Markdown
Contributor

IIRC, we have both @external and ref cells and, I think, the latter might have better support going forward since it is meaningful for both kinds of components (toplevel and non-toplevel). This might be especially true for some of the AXI stuff.

I will actually pass the hot potato to @sampsyo for confirmation.

@sampsyo
Copy link
Copy Markdown

sampsyo commented Mar 29, 2026

Interesting! I admit I don't fully understand the original bug in #10036 (i.e., I'm not sure why some component is requiring all memories to be either @external or ref). But if this works around that problem and correctly avoids externalizing internal memories, then it's probably the right thing to do? Unless there is a way to fix the root cause of that requirement.

As @rachitnigam says, the Calyx ecosystem is attempting to move away from @external. As a brief backstory, @external was invented first and (as you probably know) is necessary for writing testbenches, etc. ref is actually a generalization of @external that allows arbitrary cells (not just memories) to be provided by their callers (not just at the top level). In fact, there is an optional pass to convert one to the other:
https://github.com/calyxir/calyx/blob/052ab4168f356e6fe38aecb1422eeef0f69a63ec/calyx/opt/src/passes_experimental/external_to_ref.rs

If you're interested, it might be worth checking whether everything works if you were to stop emitting @external altogether and instead just use ref for the top-level input/output memories. I suppose I'll leave it up to you whether you think that's a more productive approach or this simpler, nearer-term thing is the right tactic for now.

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.

[SCF to Calyx] Generates external memory outside top level component

4 participants