Pin firrtl version to 1.6.0-M2

[mwachs5]

Using -SNAPSHOT does not make sense, because that means breaking changes in firrtl can break our C/I. At the moment, an upstream change has broken our documentation generation flow.

Contributor Checklist

• Did you add Scaladoc to every public function/method?
• Did you add at least one test demonstrating the PR?
• Did you delete any extraneous printlns/debugging code?
• Did you specify the type of improvement?
• Did you add appropriate documentation in docs/src?
• Did you state the API impact?
• Did you specify the code generation impact?
• Did you request a desired merge strategy?
• Did you add text to be included in the Release Notes for this change?

Type of Improvement

• bug fix

API Impact

Backend Code Generation Impact

yes, it does, but only because upstream broke its generation

Desired Merge Strategy

• Squash: The PR will be squashed and merged (choose this if you have no preference.

Release Notes

Pin firrtl version to 1.6.0-M2

Reviewer Checklist (only modified by reviewer)

• Did you add the appropriate labels?
• Did you mark the proper milestone (Bug fix: 3.4.x, [small] API extension: 3.5.x, API modification or big change: 3.6.0)?
• Did you review?
• Did you check whether all relevant Contributor checkboxes have been checked?
• Did you do one of the following when ready to merge:
  • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
  • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

[jackkoenig]

Using -SNAPSHOT does not make sense, because that means breaking changes in firrtl can break our C/I.

On the contrary, that is the point. It is a negative consequence of having closely coupled projects in different repositories, but it is intentional.

[mwachs5]

I'm confused. How is this helpful ...? We can always change this back to SNAPSHOT when that's what we want it to be. Right now, nothing will make it through chisel3 CI.

[mwachs5]

though this PR is failing C/I in the same way anyway so I'm confused what's going on here.

EDIT no actually this is failing fetching the SNAPSHOT version of treadle

[jackkoenig]

I'm confused. How is this helpful ...? We can always change this back to SNAPSHOT when that's what we want it to be. Right now, nothing will make it through chisel3 CI.

You noticed the breakage immediately and now we should fix it. It's a way of catching compatibility breaking changes immediately rather than letting them build up until you decide to bump. This is because they are such closely coupled projects, obviously this is not necessarily a development process that every project should follow.

[mwachs5]

ok, right, again we can immediately put this back when the issue is resolved. Are we OK blocking on both chisel and CIRCT fixes to do any chisel3 work until this is resolved?

[jackkoenig]

It depends on how hard the breakage is to fix but my suspicion is that it's super easy so why not just fix it?

[mwachs5]

is that it's super easy so why not just fix it?

I was told this needs a fix to llvm/CIRCT, so we need to wait for CIRCT to fix the issue then do a release then bump it here, and bumping CIRCT will probably need other work

[ekiwi]

I was told this needs a fix to llvm/CIRCT, so we need to wait for CIRCT to fix the issue then do a release then bump it here

We can also do a small fix in firrtl.

I was initially going to only add the RUW type if it is not undefined, but @seldridge asked that I add it unconditionally: chipsalliance/firrtl#2595 (comment)

We can make a PR to only conditionally emit it which should fix most of the breakage since using RUW explicitly is rare.

[mwachs5]

We can also do a small fix in firrtl.

I mean, the goal is to move forward towards correctness, so if the current SFC behavior is where we want it to be, let's leave it there and keep pushing forward towards correctness. If no one else is blocked by this CI being broken we can leave this, if we are we can figure out a way to use "the last working version" until we have more pieces that work together...

[ekiwi]

here you go: chipsalliance/firrtl#2597

[ekiwi]

I mean, the goal is to move forward towards correctness, so if the current SFC behavior is where we want it to be, let's leave it there and keep pushing forward towards correctness.

Either behavior is correct. Just one is always explicit and the other leaves out the default when possible.

[jackkoenig]

I see, I agree with @ekiwi's change because breaking Chisel with all existing releases of firtool and SFC is ill-advised unless absolutely necessary. In cases where it is necessary, then I agree with @mwachs5's proposal to temporarily pin the version to keep making progress in the meantime. chipsalliance/firrtl#2591 would also help for such things 😇

[mwachs5]

closing since SFC fix should get -SNAPSHOT working well again