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

Linker bus support #2073

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Linker bus support #2073

wants to merge 5 commits into from

Conversation

Schaeff
Copy link
Collaborator

@Schaeff Schaeff commented Nov 11, 2024

Refactor of the linker to enable creating statements both in the source and in the target of a link. Instead of creating a vector of statements, we create a (name=>namespace) map.

The linker mode is exposed in the CLI, and currently defaults to the native lookup/permutation statements.

Bus linking can be activated the following way:

cargo run pil test_data/asm/vm_to_block_unique_interface.asm --force --linker-mode bus

TODO:

  • absolute names

@Schaeff Schaeff marked this pull request as draft November 11, 2024 17:39
@Schaeff Schaeff marked this pull request as ready for review November 13, 2024 10:17
Base automatically changed from simplify-bus-interaction to main November 13, 2024 18:50
Copy link
Collaborator

@georgwiese georgwiese left a comment

Choose a reason for hiding this comment

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

Cool! Seems to work as expected.


let to_namespace = to.machine.location.clone().to_string();

// the lhs is `instr_flag { operation_id, inputs, outputs }`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment does not make sense here, does it?

Comment on lines +181 to +190
let lhs = selected(
combine_flags(from.instr_flag, from.link_flag),
ArrayLiteral {
items: op_id
.chain(from.params.inputs)
.chain(from.params.outputs)
.collect(),
}
.into(),
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's the same for lookup, could be pulled out of the if block.

Comment on lines +212 to +222
ArrayLiteral {
items: op_id
.chain(to.operation.params.inputs_and_outputs().map(|i| {
index_access(
namespaced_reference(to_namespace.clone(), &i.name),
i.index.clone(),
)
}))
.collect(),
}
.into(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would also be the same for the lookup.

@@ -154,6 +155,10 @@ enum Commands {
#[arg(long)]
backend_options: Option<String>,

/// Linker mode, deciding how to reduce links to constraints.
#[arg(long)]
linker_mode: Option<LinkerMode>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
linker_mode: Option<LinkerMode>,
#[arg(value_parser = clap_enum_variants!(LinkerMode))]
linker_mode: Option<LinkerMode>,

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.

2 participants