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

Don't run FIRRTL in FlattenSpec's ChiselStage #1493

Merged
merged 1 commit into from
Jun 23, 2020
Merged

Conversation

seldridge
Copy link
Member

Fix a bug in FlattenSpec where ChiselStage was running the FIRRTL
compiler in ChiselStage and then re-running the FIRRTL compiler. This
changes it to be like InlineSpec and to not run FIRRTL during
ChiselStage.

This produced a bug in 3.3.x, but not in the master branch slated for 3.4.0.

This was manually backported to 3.3.x as part of #1492.

Related issue: None.

Type of change: bug report

Impact: no functional change

Development Phase: implementation

Release Notes

None.

Fix a bug in FlattenSpec where ChiselStage was running the FIRRTL
compiler in ChiselStage and then re-running the FIRRTL compiler. This
changes it to be like InlineSpec and to not run FIRRTL during
ChiselStage.

This was manually backported to 3.3.x.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge requested a review from a team as a code owner June 23, 2020 19:33
@seldridge seldridge added this to the 3.4.0 milestone Jun 23, 2020
@seldridge seldridge requested a review from chick June 23, 2020 19:33
@jackkoenig
Copy link
Contributor

jackkoenig commented Jun 23, 2020

What's the [correctness] bug if we run FIRRTL twice?

@seldridge
Copy link
Member Author

Let me check. On 3.3.x CInferTypes was crashing on the low FIRRTL input. As part of the manual backport where Driver is changed to ChiselStage everywhere (#1492) I had to add the change in this PR. To try to keep everything aligned between master and 3.3.x, I forward-ported the change here.

Clarifying: there is no correctness bugfix here. master is fine.

@jackkoenig
Copy link
Contributor

Got it 👍 Thanks for the explanation.

@seldridge seldridge added the Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. label Jun 23, 2020
@mergify mergify bot merged commit 369bb6e into master Jun 23, 2020
@jackkoenig jackkoenig deleted the fix-InlineSpec-bug branch July 7, 2021 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants