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

Patch for Spade -> Filament #460

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Conversation

ethanuppal
Copy link
Member

@ethanuppal ethanuppal commented Sep 16, 2024

NOT READY FOR REVIEW

input wire logic [WIDTH-1:0] left,
output wire logic [WIDTH-1:0] out
);
assign out = left + right;
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here? right is not defined...

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still making changes, so this is a draft PR and not ready for review. There's a lot of broken stuff right now.

Copy link
Member

Choose a reason for hiding this comment

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

I think you might have requested a review. If that was unintentional, then tag me when ready for review.

Copy link
Member Author

Choose a reason for hiding this comment

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

It might have automatically done that

@@ -32,6 +32,16 @@ impl From<&cmdline::Opts> for Resolver {
}

impl Resolver {
pub fn new(lib: Vec<PathBuf>, input: PathBuf) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

What's the use case for this method?

Copy link
Member Author

Choose a reason for hiding this comment

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

I want to make a resolver without constructing an instance of the filament command line options.


extern "core.sv" {
comp Wire[WIDTH]<'G, 'L>(
Copy link
Collaborator

@gabizon103 gabizon103 Sep 17, 2024

Choose a reason for hiding this comment

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

What would this be for? I imagine it would be for renaming ports, but it can probably be done with Filament bundles.

If you have some port p0, I guess you would want to pass it to some instantiation add_in := new Wire[W]<'G>(p0) to get a port named add_in.out that would be passed into an adder?

If you have p0, you can define a bundle bundle add_in[1]: ['G, 'G+1] W and then make the assignment add_in{0} = p0. Then you'd use add_in{0} wherever. To me, this is also more readable.

Copy link
Member Author

Choose a reason for hiding this comment

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

It makes implementing Spade's Alias operation easier.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should move Spade-specific stuff to a spade.fil file instead instead of adding things to the standard library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I was thinking that too

@ethanuppal
Copy link
Member Author

Ok don't actually think I need to expose SigMap? but I'll keep it in for now in case I do

@ethanuppal ethanuppal force-pushed the main branch 2 times, most recently from 5a52b97 to 538d900 Compare September 23, 2024 21:13
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