Chisel 7 Support - ChiselAnnotation going away

[tymcauley]

With chipsalliance/chisel#4717, Chisel has removed the ChiselAnnotation API, which breaks a number of spots throughout rocket-chip:

• rocket-chip/src/main/scala/util/ElaborationArtefactAnnotation.scala

  Lines 41 to 49 in ac5192c

  	object ElaborationArtefactAnnotation {
  	/** Emits [[ElaborationArtefactAnnotation]] for the given filename extension and tokens.
  	*/
  	def annotate(filename: String, tokens: => Seq[Token]): Unit = {
  	chisel3.experimental.annotate(new ChiselAnnotation {
  	def toFirrtl = ElaborationArtefactAnnotation(filename, tokens.toList)
  	})
  	}
  	}

• rocket-chip/src/main/scala/rocket/DCache.scala

  Lines 26 to 31 in ac5192c

  	// TODO: delete this trait once deduplication is smart enough to avoid globally inlining matching circuits
  	trait InlineInstance { self: chisel3.experimental.BaseModule =>
  	chisel3.experimental.annotate(
  	new chisel3.experimental.ChiselAnnotation {
  	def toFirrtl: firrtl.annotations.Annotation = firrtl.passes.InlineAnnotation(self.toNamed) } )
  	}

• Many uses within https://github.com/chipsalliance/rocket-chip/blob/ac5192c76646569ebd47d247b33cd69d6f01ce0c/src/main/scala/util/Annotations.scala

Can we replace the custom DCache InlineInstance trait with either chisel3.util.experimental.InlineInstance (in Chisel 6.6.0) or InlineInstanceAllowDedup (in Chisel 7)?

Do the other annotations have any consumer, or would they be harmless to remove?

[jerryz123]

It looks like ElaborationArtefactAnnotation is also unused? I think all the unused Annotations can be removed.

InlineInstance is useful, but I agree that should be changed to experimental.InlineInstance.

[tymcauley]

It looks like ElaborationArtefactAnnotation is also unused? I think all the unused Annotations can be removed.

InlineInstance is useful, but I agree that should be changed to experimental.InlineInstance.

Regarding DCache's InlineInstance, should we try to use chisel3.util.experimental.InlineInstanceAllowDedup, since that doesn't block dedup, like chisel3.util.experimental.InlineInstance does?

[jerryz123]

Yeah, good point. It would be unintuitive for inlined instances to block dedup

[tymcauley]

I'll submit a PR to get rid of most of the annotations except for DCache's InlineInstance. That PR will be Chisel-6 compatible. Then I'll make another which replaces DCache's InlineInstance

[tymcauley]

Okay, this PR contains the removal of most of rocket-chip's annotations, aside from the DCache one: #3723

It impacts APIs that are used by other projects, so I've submitted PRs to those too (and they're listed in the rocket-chip PR).

[jackkoenig]

I want to clarify that, while Chisel 7 removes ChiselAnnotation, it's still possible to annotate things, you just have to go directly to FirrtlAnnotation¹. That being said, since it's basically impossible to implement user-defined annotations in CIRCT, there's a lot less utility in them than there once was anyway--it's best to use upstream annotations where possible.

Footnotes

1. Admittedly this API is not released but will be in 6.7.0. I can prioritize getting 6.7.0 out if it helps y'all at all. ↩

[tymcauley]

I want to clarify that, while Chisel 7 removes ChiselAnnotation, it's still possible to annotate things, you just have to go directly to FirrtlAnnotation1. That being said, since it's basically impossible to implement user-defined annotations in CIRCT, there's a lot less utility in them than there once was anyway--it's best to use upstream annotations where possible.

Footnotes

1. Admittedly this API is not released but will be in 6.7.0. I can prioritize getting 6.7.0 out if it helps y'all at all. [↩](#user-content-fnref-1-a3f223b2745d92b1488adc93a6564054)

Ya, that might help us resolve ucb-bar/constellation#81 and chipsalliance/diplomacy#18

[tymcauley]

I want to clarify that, while Chisel 7 removes ChiselAnnotation, it's still possible to annotate things, you just have to go directly to FirrtlAnnotation1. That being said, since it's basically impossible to implement user-defined annotations in CIRCT, there's a lot less utility in them than there once was anyway--it's best to use upstream annotations where possible.

Footnotes

1. Admittedly this API is not released but will be in 6.7.0. I can prioritize getting 6.7.0 out if it helps y'all at all. [↩](#user-content-fnref-1-a3f223b2745d92b1488adc93a6564054)

@jackkoenig I don't see the FirrtlAnnotation API in Chisel right now, all I see are calls to chisel3.experimental.annotate(...).

[jackkoenig]

I've tagged v6.7.0, the artifacts should be on Maven in ~2 hours: https://github.com/chipsalliance/chisel/releases/tag/v6.7.0

@tymcauley the new chisel3.experimental.annotate takes => Seq[firrtl.annotations.Annotation] as its 2nd argument list, for example¹:

  def addResource(blackBoxResource: String): Unit = {
    chisel3.experimental.annotate(self)(Seq(BlackBoxInlineAnno.fromResource(blackBoxResource, self.toNamed)))
  }

Also see the PR (chipsalliance/chisel#4643) that changed from the old to new for an example.

Footnotes

1. https://github.com/chipsalliance/chisel/blob/425bdd2bd6132de684d0fdebe5f5a09aca61acb5/src/main/scala/chisel3/util/BlackBoxUtils.scala#L47 ↩

[tymcauley]

Yup, we can use the new annotation API from chipsalliance/chisel#4643 to fix the remaining ChiselAnnotation in DCache, I'll submit a PR for that.

[tymcauley]

Resolved by #3724