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

Add implicit CompileOptions to Record and Bundle. Fixes #495 #595

Merged
merged 2 commits into from
May 10, 2017

Conversation

jackkoenig
Copy link
Contributor

Helps distinguish between Records/Bundles defined in Chisel._ vs.
chisel3.. Also override compilationOptions when bulk connecting
Records/Bundles defined in Chisel.
. This allows Records/Bundles defined
in Chisel._ code to be correctly bulk connected in chisel3._ code.

Helps distinguish between Records/Bundles defined in Chisel._ vs.
chisel3._. Also override compilationOptions when bulk connecting
Records/Bundles defined in Chisel._. This allows Records/Bundles defined
in Chisel._ code to be correctly bulk connected in chisel3._ code.
@jackkoenig jackkoenig requested a review from ducky64 April 26, 2017 22:32
Copy link
Contributor

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

lgtm

@jackkoenig
Copy link
Contributor Author

jackkoenig commented Apr 27, 2017 via email

@ducky64
Copy link
Contributor

ducky64 commented Apr 27, 2017

According to https://stackoverflow.com/questions/5598085/where-does-scala-look-for-implicits, first looks in current scope. I think if there are multiple options in any of those priorities, the compiler will error out with an ambiguous implicit. I guess it's helpful that Record methods don't have implicit arguments, though superclass methods do.

@jackkoenig
Copy link
Contributor Author

It seems that this isn't exactly doing what I wanted, so I'm taking another look.

@jackkoenig
Copy link
Contributor Author

As discussed last week, not going to let the perfect be the enemy of the good. I'm going to merge this when updating passes Jenkins.

This currently issues a Firrtl bulk connect whenever a Chisel._ Record/Bundle is used in chisel3._ code. The better proposed solution is to still do the standard chisel3._ bulk connect walk through leaf elements, but be a bit more permissive for Chisel._ bundles (eg. no explicit direction is Output).

@jackkoenig
Copy link
Contributor Author

jackkoenig commented May 10, 2017

There appears to be a compilation issue with rocket-chip, debugging.

@jackkoenig
Copy link
Contributor Author

False alarm, we good

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