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

[3/x] Link Miden packages without cross-context calls lifting/lowering #353

Merged
merged 17 commits into from
Dec 16, 2024

Conversation

greenhat
Copy link
Contributor

@greenhat greenhat commented Nov 14, 2024

Close #351
Close #346

Ref #303

This PR is stacked on the #356 and should be merged after it

This PR links P2ID note script with the basic wallet Miden package, ignoring the lifting/lowering of the cross-context calls. The cross-context calls will be addressed in the subsequent PR(s).
Since this PR contains quite a few changes in various parts of the compiler, I suggest going by commits for the review process.

This PR makes the following decisions:

  • Skip registering already registered modules with the assembler to avoid duplicate registration of the intrinsics, etc. modules;
  • Recover Wasm CM interfaces as module names from the function names after assembling the library from (workaround for multiple interfaces in one Wasm core module and assembler using module name when building library exports); In the subsequent PRs Component will be used in compiler pipeline instead of Module and lifting/lowering will be done in separate modules so most likely this workaround will not be necessary
  • In library exports, #anon namespace is used with Wasm CM interface (miden:basic-wallet/[email protected]) resulting in #anon::miden:basic-wallet/[email protected] string as the export module name;
  • Populate Linker::allow_missing with linked library's exports (with removed #anon namespace);

@greenhat
Copy link
Contributor Author

@bitwalker We need high-level type signatures to link a Miden package - #354
What do you think?

@greenhat greenhat force-pushed the greenhat/i346-link-pkg-gen-miden-abi branch 3 times, most recently from 3d9b15e to 56d45c1 Compare November 26, 2024 10:16
@greenhat greenhat changed the base branch from greenhat/i346-link-lib-package to greenhat/delete-cargo-component-based-tests November 26, 2024 10:16
@greenhat greenhat changed the title [2/2] Link package, preparation for cross-context call generation [3/x] Link package, preparation for cross-context call generation Nov 26, 2024
@greenhat greenhat changed the title [3/x] Link package, preparation for cross-context call generation [3/x] Link Miden package without cross-context calls lifting/lowering Nov 26, 2024
@greenhat greenhat changed the title [3/x] Link Miden package without cross-context calls lifting/lowering [3/x] Link Miden packages without cross-context calls lifting/lowering Nov 26, 2024
@greenhat greenhat force-pushed the greenhat/delete-cargo-component-based-tests branch from 2466128 to 73262e3 Compare November 26, 2024 13:16
@greenhat greenhat force-pushed the greenhat/i346-link-pkg-gen-miden-abi branch from 56d45c1 to 6745c3f Compare November 26, 2024 13:17
@greenhat greenhat marked this pull request as ready for review December 16, 2024 13:12
@greenhat greenhat requested a review from bitwalker December 16, 2024 13:12
Base automatically changed from greenhat/delete-cargo-component-based-tests to next December 16, 2024 19:08
@bitwalker
Copy link
Contributor

@greenhat Yes, high-level type signatures in Miden are something I'll be adding starting in a week or possibly at the start of the new year. Once added, they'll exist at the same level of abstraction as core Wasm signatures (albeit more useful, since not everything will be reduced to i32), and we'll rely on WIT + component types to tell us the high-level type signatures in the Canonical ABI for any component exports authored in Miden Assembly.

A couple notes about some of the decisions made here:

  • Rather than using the Anon namespace, we should translate symbol names as described in the Component Translation discussion. The gist is that the WIT package name becomes the LibraryNamespace (specifically the User variant), sans version, items exported from the component use the export name as the first non-namespace path component, and so on (i.e. functions exported from an exported interface are additional components). Future changes to LibraryPath will make the syntax for fully-qualified paths a bit more like WIT identifiers, but for now the main thing we want to do is translate them in a way that retains useful semantics in Miden Assembly. We've largely already discussed this, but I'm just reiterating it here since it is relevant to decisions documented in this PR
  • Likewise, the way in which module/function names are recovered I believe will need to be changed, but we can do that when merging the new IR into the frontend. This is also described in the Component Translation discussion, but essentially we can recover the original WIT interfaces (and their names/exports) using the component instances exported from the top-level component
  • Once the new IR changes are merged, the Linker will no longer require us to explicitly allow "missing" symbols - so long as those symbols are accounted for via a dependency. The exception will be intrinsics of course, but those will be special-cased anyway. That doesn't affect anything here, just noting it for posterity.

I'm going to review/merge this under the assumption that we'll address any of the above in future PRs, but wanted to bring them up to ensure they are something we don't forget to deal with once the IR refactoring is merged (or as part of merging it).

let mut mast_forest = lib.mast_forest().as_ref().clone();
mast_forest.advice_map_mut().extend(advice_map);
let converted_exports = recover_wasm_cm_interfaces(&lib);
let lib = CompiledLibrary::new(Arc::new(mast_forest), converted_exports)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: When wiring up HIR2 to the frontend, the lowering from Wasm to HIR will need to create Interface operations in the top-level Component which contain Function ops corresponding to the exports of component instances in the original Wasm component. The Component operation will also contain component-level function exports and Modules corresponding to the core Wasm modules present in the original Wasm component. See Component Translation for further details.

The new codegen crate no longer has its own Miden Assembly IR, so the Library and Program structs no longer exist. To the extent that some of this stuff still exists (by which I mean things like assembling to MAST, late-stage linking and the like), it will either be done in the new Linker (which is much simpler than the current one, and runs just prior to codegen, rather than as a initial part of lowering into HIR), or in the compiler driver (i.e. in an appropriate Stage of the compiler). I expect that a lot of this will no longer be necessary though, since the new representation is more faithful to the Component Model, so fewer contortions are required to lower into Miden Assembly than we currently are forced to do.

@bitwalker bitwalker merged commit 249271d into next Dec 16, 2024
5 checks passed
@bitwalker bitwalker deleted the greenhat/i346-link-pkg-gen-miden-abi branch December 16, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants