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

Implement support for vendoring procedures in the assembler #1435

Open
bobbinth opened this issue Aug 7, 2024 · 3 comments
Open

Implement support for vendoring procedures in the assembler #1435

bobbinth opened this issue Aug 7, 2024 · 3 comments
Labels
assembly Related to Miden assembly
Milestone

Comments

@bobbinth
Copy link
Contributor

bobbinth commented Aug 7, 2024

Currently, when the assembler assembles libraries/programs, it assumes that all external libraries will be available to the processor to execute the assembled code. However, this may not be true in all instances.

So, when adding external libraries to the assembler, we need to specify whether a given library is expected to be available to the processor or should be vendored (i.e., copied into the assembled program/library).

@bobbinth bobbinth added the assembly Related to Miden assembly label Aug 7, 2024
@bobbinth bobbinth added this to the v0.12.0 milestone Aug 7, 2024
@bitwalker
Copy link
Contributor

This is fairly straightforward to implement, we'll need another API for adding vendored libraries, and for those libraries we'll want to take ownership over the CompiledLibrary (or clone it), we'd track these separately from non-vendored libraries. Then, when we encounter a reference to a procedure whose definition is in a vendored library, we do a depth-first, post-order traversal of the MastForest of that library, from the procedure root, copying all of the nodes (after all their children are processed) into the MastForest being built, rewriting any MastNodeIds in the process. This will ensure that we only vendor what is strictly needed. We can of course skip the traversal for any node that is already in the forest, so long as that node is not External (unless the External node is not present in any of the vendored libraries).

@bobbinth
Copy link
Contributor Author

bobbinth commented Aug 7, 2024

I think one challenge we'd need to address about vendored libraries is that it may be expensive to proactively clone all MAST forests (we kind of do this right now for non-vendored libraries, but I see this more like a temporary solution). I think building a set of ModuleInfo objects for all modules that may potentially be relevant to the program/library that is being assembled is probably cheap enough because ModuleInfo struct is pretty light-weight. But we'd probably want to avoid doing the same for all potentially relevant MAST forests.

I think we can address this by introducing something like AssemblyContext I mentioned in #1433 (comment). Here, the assembler would never actually take ownership of libraries, but would use the context to build ModuleInfo structs and extract MASTs only for relevant procedures.

@bitwalker
Copy link
Contributor

I suspect we could easily refactor the API to take CompiledLibrary by reference (and in general, it probably makes sense to always wrap them in Rc or Arc anyway, as they are immutable, and that would vastly simplify this issue), since almost by definition they would outlive the Assembler due to its one-shot nature.

The ModuleGraph itself doesn't need ownership of them, only a reference, the main pain point to using plain references versus smart pointers is the need to refactor a bunch of APIs to have explicit lifetimes. So my first instinct would be to evaluate if we can just require the use of reference-counted CompiledLibrary references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assembly Related to Miden assembly
Projects
None yet
Development

No branches or pull requests

2 participants